Revert "libc/misc: Mark lock functions as weak" #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 196ddba. Static library archives don't preserve information about which symbols are weak, so this couldn't work anyway.
asiekierka
merged commit e6dde28
into
WonderfulToolchain:wonderful
2 of 4 checks passed
asiekierka
pushed a commit
that referenced
this pull request
May 2, 2024
We have to tell GDB about the correct "return" address (mepc) as well as
the saved stack pointer to continue unwinding beyond the current frame.
With this change and a __builtin_trap added to fclose(), I get the
following mostly-sensible (unwinding fails one frame up) backtrace
when breaking at the end of _trap:
```
$ gdb -q test/test-fopen '-ex=b crt0.c:169' '-ex=target remote :1234' -ex=c -ex=bt
Reading symbols from test/test-fopen...
Breakpoint 1 at 0x800001c0: file ../../picolibc/picocrt/machine/riscv/crt0.c, line 169.
Remote debugging using :1234
0x0000000000001000 in ?? ()
Continuing.
Breakpoint 1, _trap () at ../../picolibc/picocrt/machine/riscv/crt0.c:169
169 __asm__("j _ctrap");
#0 _trap () at ../../picolibc/picocrt/machine/riscv/crt0.c:169
#1 0x00000000800013d0 in fclose (f=0x0) at ../../picolibc/newlib/libc/tinystdio/fclose.c:39
#2 0x0000000000000000 in ?? ()
Backtrace stopped: frame did not save the PC
```
asiekierka
pushed a commit
that referenced
this pull request
May 2, 2024
Currently the trap frame will overwrite previously saved registers, so the
GDB backtrace is somewhat unreliable. Instead of writing the trap frame to
the top of the stack write it to the end of the heap instead. While this
could still corrupt some state it is hopefully still unused and will
definitely not overwrite previous return addresses. In the previous example
I now get a backtrace all the way up to main():
```
$ gdb -q test/test-fopen '-ex=b crt0.c:169' '-ex=target remote :1234' -ex=c -ex=bt
Reading symbols from test/test-fopen...
Breakpoint 1 at 0x800001c0: file ../../picolibc/picocrt/machine/riscv/crt0.c, line 168.
Remote debugging using :1234
0x0000000000001000 in ?? ()
Continuing.
Breakpoint 1, _trap () at ../../picolibc/picocrt/machine/riscv/crt0.c:169
169 __asm__("j _ctrap");
#0 _trap () at ../../picolibc/picocrt/machine/riscv/crt0.c:168
#1 0x00000000800013d0 in fclose (f=0x804008c0) at ../../picolibc/newlib/libc/tinystdio/fclose.c:39
#2 0x0000000080000680 in main () at ../../picolibc/test/test-fopen.c:85
```
asiekierka
pushed a commit
that referenced
this pull request
May 2, 2024
This ensures that running `b _ctrap; c; bt` inside gdb shows the right
backtrace. If we jump without setting ra gdb gets confused about the
backtrace since we don't have the right cfi directives inside ctrap.
This does not result in any downsides since ra has already been saved.
Previously we had to set a breakpoint just before this jump to get a
correct backtrace, now it's possible to set a breakpoint in _ctrap
instead:
```
Reading symbols from test/test-fopen...
Breakpoint 1 at 0x80000058: file ../../picolibc/picocrt/machine/riscv/crt0.c, line 84.
Remote debugging using :1234
0x0000000000001000 in ?? ()
Continuing.
Breakpoint 1, _ctrap (fault=0x805feee8) at ../../picolibc/picocrt/machine/riscv/crt0.c:84
84 printf("RISCV fault\n");
#0 _ctrap (fault=0x805feee8) at ../../picolibc/picocrt/machine/riscv/crt0.c:84
#1 0x00000000800001c4 in _trap () at ../../picolibc/picocrt/machine/riscv/crt0.c:168
#2 0x00000000800013d0 in fclose (f=0x804008c0) at ../../picolibc/newlib/libc/tinystdio/fclose.c:39
picolibc#3 0x0000000080000680 in main () at ../../picolibc/test/test-fopen.c:85
(gdb)
```
asiekierka
pushed a commit
that referenced
this pull request
Sep 1, 2024
- The PRU newlib port is supposed to implement case #2 as described in newlib/libc/include/reent.h . Thus drop the defining of MISSING_SYSCALL_NAMES. - libgloss/configure: Regenerate. - libgloss/libnosys/acinclude.m4: Do not define MISSING_SYSCALL_NAMES for pru. Signed-off-by: Dimitar Dimitrov <dimitar@dinux.eu>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 196ddba.
Static library archives don't preserve information about which symbols are weak, so this couldn't work anyway.