Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions libc/src/setjmp/x86_64/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
add_entrypoint_object(
setjmp
SRCS
setjmp.cpp
setjmp.S
HDRS
../setjmp_impl.h
DEPENDS
libc.include.setjmp
COMPILE_OPTIONS
-O3
-fno-omit-frame-pointer
)

add_entrypoint_object(
Expand Down
50 changes: 50 additions & 0 deletions libc/src/setjmp/x86_64/setjmp.S
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//===-- Implementation of setjmp --------------------------------*- ASM -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#define paste(ns) _ZN22 ## ns ## 6setjmpEP9__jmp_buf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest an extern C name which includes the expansion of LIBC_NAMESPACE in the name, rather than manual C++ name-mangling. If you want to do C++ name-mangling you'll need to figure out how to generate the "22" since that needs to be the length of the namespace.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an extern C name which includes the expansion of LIBC_NAMESPACE in the name

I'm sorry I don't follow the suggestion. Can you please "break out the crayons" for me?

#define expand(x) paste(x)
#define LIBC_NAMESPACE_setjump expand(LIBC_NAMESPACE)
// aka _ZN22__llvm_libc_19_0_0_git6setjmpEP9__jmp_buf
// aka __llvm_libc_19_0_0_git::setjmp(__jmp_buf*)

// Brittle! Changing the layout of __jmp_buf will break this!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move into a header file, so setjmp_test can include?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#define RBX_OFFSET 0
#define RBP_OFFSET 8
#define R12_OFFSET 16
#define R13_OFFSET 24
#define R14_OFFSET 32
#define R15_OFFSET 40
#define RSP_OFFSET 48
#define RIP_OFFSET 56

.global setjump
.global LIBC_NAMESPACE_setjump

.type setjump, @function
.type LIBC_NAMESPACE_setjump, @function

setjump:
Copy link
Member

@jyknight jyknight Apr 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this public symbol need to be conditioned on LIBC_COPT_PUBLIC_PACKAGING?

Maybe something like:

#ifdef LIBC_COPT_PUBLIC_PACKAGING
#define LIBC_ASM_PUBLIC_ALIAS(alias, orig) \
  .globl  alias SEPARATOR \
  .type   alias,@function SEPARATOR \
  .set alias, orig
#else
#define LIBC_ASM_PUBLIC_ALIAS(alias, orig)
#endif

LIBC_ASM_PUBLIC_ALIAS(setjmp, LIBC_NAMESPACE_setjmp)

(Note: in the above .size isn't needed, as it gets copied over automatically)

[edit: added SEPARATOR, since otherwise it'd all get mushed together on one line]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably! Good find wrt SEPARATOR.

Orthogonal: Why does apple+aarch64 need %% as separator? 🤮

LIBC_NAMESPACE_setjump:

mov %rbx, RBX_OFFSET(%rdi)
mov %rbp, RBP_OFFSET(%rdi)
mov %r12, R12_OFFSET(%rdi)
mov %r13, R13_OFFSET(%rdi)
mov %r14, R14_OFFSET(%rdi)
mov %r15, R15_OFFSET(%rdi)
lea 8(%rsp), %rax
mov %rax, RSP_OFFSET(%rdi)
mov (%rsp), %rax
mov %rax, RIP_OFFSET(%rdi)
xor %eax, %eax
ret

.size setjump, . - setjump
.size LIBC_NAMESPACE_setjump, . - LIBC_NAMESPACE_setjump

.section .note.GNU-stack, "", @progbits
56 changes: 0 additions & 56 deletions libc/src/setjmp/x86_64/setjmp.cpp

This file was deleted.

11 changes: 11 additions & 0 deletions libc/test/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,14 @@ add_libc_test(
DEPENDS
libc.include.llvm-libc-macros.stdckdint_macros
)

add_libc_test(
setjmp_test
SUITE
libc_include_tests
SRCS
setjmp_test.cpp
DEPENDS
libc.include.llvm-libc-macros.offsetof_macro
libc.include.llvm-libc-types.jmp_buf
)
29 changes: 29 additions & 0 deletions libc/test/include/setjmp_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//===-- Unittests for setjmp ----------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDSList-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include "include/llvm-libc-macros/offsetof-macro.h"
#include "include/llvm-libc-types/jmp_buf.h"
#include "test/UnitTest/Test.h"

// If this test fails, then *_OFFSET macro definitions in
// libc/src/setjmp/x86_64/setjmp.S need to be fixed. This is a simple change
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libc/src/setjmp/x86_64/setjmp.h is the updated source location

// detector.
TEST(LlvmLibcSetjmpTest, JmpBufLayout) {
#ifdef __x86_64__
ASSERT_EQ(offsetof(__jmp_buf, rbx), 0UL);
ASSERT_EQ(offsetof(__jmp_buf, rbp), 8UL);
ASSERT_EQ(offsetof(__jmp_buf, r12), 16UL);
ASSERT_EQ(offsetof(__jmp_buf, r13), 24UL);
ASSERT_EQ(offsetof(__jmp_buf, r14), 32UL);
ASSERT_EQ(offsetof(__jmp_buf, r15), 40UL);
ASSERT_EQ(offsetof(__jmp_buf, rsp), 48UL);
ASSERT_EQ(offsetof(__jmp_buf, rip), 56UL);
ASSERT_EQ(sizeof(__jmp_buf), 64UL);
ASSERT_EQ(alignof(__jmp_buf), 8UL);
#endif // __x86_64__
}