Skip to content

linx: bring up dynamic glibc printf path#9

Open
touzicongbupianren wants to merge 2 commits into
LinxISA:masterfrom
touzicongbupianren:lihan-glibc-user-bringup-pr
Open

linx: bring up dynamic glibc printf path#9
touzicongbupianren wants to merge 2 commits into
LinxISA:masterfrom
touzicongbupianren:lihan-glibc-user-bringup-pr

Conversation

@touzicongbupianren

Copy link
Copy Markdown

This PR brings up the minimal Linx glibc runtime path needed for dynamic qemu-user printf execution.

Changes:

  • Add Linx setjmp/longjmp assembly support.
  • Add rtld-specific setjmp support.
  • Add thread pointer and stack/pointer guard support.
  • Adjust Linx syscall return/error handling.
  • Remove temporary Linx rtld diagnostics that polluted stdout.

Validation:

  • glibc G1b build passes.
  • ld.so and libc.so are generated.
  • qemu-user can run a dynamically linked Linx printf program through /lib/ld.so.1 and libc.so.6.
  • stdout is clean and stderr is empty.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request cleans up debug tracing in the dynamic linker and implements core architecture-specific features for Linx, including setjmp/longjmp, thread pointer access, stack/pointer guards, and syscall macro refinements. Feedback highlights critical issues with stack and pointer guard macros in tls.h and stackguard-macros.h that use incorrect negative array indexing ([-1]), which can lead to memory corruption and broken stack protection. Additionally, the __sigsetjmp implementation in setjmp.S needs to be updated to properly handle the savemask argument and call __sigjmp_save to comply with POSIX and glibc standards.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread sysdeps/linx/nptl/tls.h
Comment on lines +93 to +105
# define THREAD_SET_STACK_GUARD(value) \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].stack_guard = (value))
# define THREAD_COPY_STACK_GUARD(descr) \
(((tcbhead_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))[-1].stack_guard \
= ((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].stack_guard)

# define THREAD_GET_POINTER_GUARD() \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].pointer_guard)
# define THREAD_SET_POINTER_GUARD(value) \
(THREAD_GET_POINTER_GUARD () = (value))
# define THREAD_COPY_POINTER_GUARD(descr) \
(((tcbhead_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))[-1].pointer_guard \
= THREAD_GET_POINTER_GUARD ())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The macros THREAD_SET_STACK_GUARD, THREAD_COPY_STACK_GUARD, THREAD_GET_POINTER_GUARD, THREAD_SET_POINTER_GUARD, and THREAD_COPY_POINTER_GUARD are using [-1] to access stack_guard and pointer_guard.

Since stack_guard and pointer_guard are defined as members inside the tcbhead_t structure (at positive offsets), using [-1] will subtract sizeof(tcbhead_t) from the thread pointer, thereby accessing memory before the TCB. In TLS Variant I, this memory corresponds to the thread's static TLS segment, leading to silent memory corruption of thread-local variables.

Furthermore, the compiler's stack protector expects the stack guard to be at a positive offset (e.g., tp + 16), so initializing it at a negative offset (tp - 16) will leave the actual stack guard uninitialized (or zero), completely breaking stack protection.

Please use direct member access (->) instead of array indexing ([-1]).

# define THREAD_SET_STACK_GUARD(value) \
  (((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))->stack_guard = (value))
# define THREAD_COPY_STACK_GUARD(descr) \
  (((tcbhead_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))->stack_guard \
   = ((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))->stack_guard)

# define THREAD_GET_POINTER_GUARD() \
  (((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))->pointer_guard)
# define THREAD_SET_POINTER_GUARD(value) \
  (THREAD_GET_POINTER_GUARD () = (value))
# define THREAD_COPY_POINTER_GUARD(descr) \
  (((tcbhead_t *) ((char *) (descr) + TLS_PRE_TCB_SIZE))->pointer_guard \
   = THREAD_GET_POINTER_GUARD ())

Comment on lines +4 to +8
#define STACK_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].stack_guard)

#define POINTER_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].pointer_guard)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Similar to the issue in nptl/tls.h, the STACK_CHK_GUARD and POINTER_CHK_GUARD macros are using [-1] to access the guards. Since these fields are members of tcbhead_t, they should be accessed directly using -> to avoid reading from the wrong memory location (which corrupts/reads from the static TLS area).

Suggested change
#define STACK_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].stack_guard)
#define POINTER_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))[-1].pointer_guard)
#define STACK_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))->stack_guard)
#define POINTER_CHK_GUARD \
(((tcbhead_t *) ((char *) READ_THREAD_POINTER () - TLS_TCB_OFFSET))->pointer_guard)

Comment thread sysdeps/linx/setjmp.S
Comment on lines +51 to +53
ENTRY (__sigsetjmp)
SAVE_JMPBUF ()
END (__sigsetjmp)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

In libc.so, __sigsetjmp is required to save the signal mask if the savemask argument (passed in a1) is non-zero. Currently, __sigsetjmp simply calls SAVE_JMPBUF() and returns 0 directly, completely ignoring savemask and never calling __sigjmp_save.

While this minimal implementation might be sufficient for basic rtld and simple printf execution, it violates POSIX and glibc standards for user-space execution and will cause sigsetjmp/siglongjmp to behave incorrectly in user programs.

Consider implementing a proper __sigsetjmp that saves the registers and then tail-calls __sigjmp_save to handle the signal mask.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant