Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

Remove the .data section from RISC-V's default linker script addon #47

Closed
wants to merge 3 commits into from
Closed

Remove the .data section from RISC-V's default linker script addon #47

wants to merge 3 commits into from

Conversation

palmer-dabbelt
Copy link
Contributor

I'm actually not sure why this is here: .data isn't read-only and
shouldn't be in the INITIAL_READONLY_SECTIONS variable. This causes a
handful of sections that should be read-only to actually be read-write
when loaded, which breaks things like "-z relro".

Removing this causes the default .data mapping to be used, which comes
from upstream. As far os I can tell, there was no reason we were ever
supposed to have our own .data in the first place, and no other
architectures have ever done it this way.

I tested this still builds and boots Busybox on Linux.

Thanks to Alan Morda for finding the problem.

ld/ChangeLog

2016-12-29 Palmer Dabbelt [email protected]

    * emulparams/elf32lriscv-defs.sh (INITIAL_READONLY_SECTIONS):
    Remove the RISC-V specific .data section mapping.

palmer-dabbelt and others added 2 commits December 21, 2016 14:12
@palmer-dabbelt
RISC-V GDB Port
cb8604c 
ChangeLog:

* New RISC-V GDB port
@kito-cheng @palmer-dabbelt
Add support for the RISC-V Q extension
8c5f683 
The RISC-V ISA manual defines a Q instruction set extension that enables
hardware support for quad precision IEEE floating point.  This patch
enables support for the Q extension: it declares the Q instruction
encoding macros, adds support for the various Q instructions to the
encoding table, whitelists "q" as an ISA extension, and sets
FLOAT_ABI_QUAD when the Q extension is enabled.

gas/ChangeLog

2016-12-21  Kito Cheng <[email protected]>

        * config/tc-riscv.c (riscv_set_arch): Whitelist the "q" ISA
        extension.
        (riscv_after_parse_args): Set FLOAT_ABI_QUAD when the Q ISA is
        enabled and no other ABI is specified.

include/ChangeLog

2016-12-21  Kito Cheng <[email protected]>

        * opcode/riscv-opc.h: Add support for the "q" ISA extension.

opcodes/ChangeLog

2016-12-21  Kito Cheng <[email protected]>

        * riscv-opc.c (riscv-opcodes): Add support for the "q" ISA
        extension.
        * riscv-opcodes/all-opcodes: Likewise.
kito-cheng
kito-cheng reviewed Dec 30, 2016
View reviewed changes
ld/emulparams/elf32lriscv-defs.sh
@@ -29,7 +29,6 @@ SDATA_START_SYMBOLS="_gp = . + 0x800;
# Place the data section before text section. This enables more compact
# global variable access for RVC code via linker relaxation.
INITIAL_READONLY_SECTIONS="
.data : { *(.data) *(.data.*) *(.gnu.linkonce.d.*) }
.rodata : { *(.rodata) *(.rodata.*) *(.gnu.linkonce.r.*) }
.srodata : { ${SDATA_START_SYMBOLS} }
.sdata : { *(.sdata .sdata.* .gnu.linkonce.s.*) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

sdata mean small data, so I think you need remove it too.

@aswaterman
Copy link
Contributor

Kito's right. But I suggest just deleting the entire INITIAL_READONLY_SECTIONS statement. It is a micro-optimization for RVC that adds to the "weirdness budget" but does not help much. I did that in this commit: riscv-collab/riscv-gnu-toolchain@97951e3

I'll force-push over this branch with a partial revert of that commit. (We don't want to move the text section again--it's fine at 0x10000.)

@palmer-dabbelt
Copy link
Contributor Author

@aswaterman I just pushed Kito's suggestion, IDK if I overwrote whatever you were doing. Can you just make this what you want and then ping me?

@palmer-dabbelt
Copy link
Contributor Author

@aswaterman It looks like your patch breaks the glibc build:

mv -f /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/libc.so.6.new /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/libc.so.6
riscv64-unknown-linux-gnu-gcc -march=rv32ima -mabi=ilp32 -nostdlib -nostartfiles -o /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/iconv/iconvconfig    -Wl,-z,combreloc -Wl,-z,relro -Wl,--hash-style=both /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/csu/crt1.o /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/csu/crti.o `riscv64-unknown-linux-gnu-gcc -march=rv32ima -mabi=ilp32  --print-file-name=crtbegin.o` /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/iconv/iconvconfig.o /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/iconv/strtab.o /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/iconv/xmalloc.o /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/iconv/hash-string.o  -Wl,-dynamic-linker=/lib32/ilp32/ld.so.1 -Wl,-rpath-link=/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/math:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/elf:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/dlfcn:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/nss:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/nis:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/rt:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/resolv:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/crypt:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/mathvec:/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/nptl /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/libc.so.6 /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/libc_nonshared.a -Wl,--as-needed /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/elf/ld.so -Wl,--no-as-needed -lgcc  `riscv64-unknown-linux-gnu-gcc -march=rv32ima -mabi=ilp32  --print-file-name=crtend.o` /scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/csu/crtn.o
/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/build-linux-multilib/build-glibc-linux-rv32ima-ilp32/csu/crt1.o: In function `.L0 ':
/scratch/palmer.dabbelt/work/upstream/riscv-gnu-toolchain/riscv-glibc/csu/../sysdeps/riscv/start.S:58: undefined reference to `_gp'

Do we need to keep the SDATA_START_SYMBOLS line? That seems to be where we get _gp from.

@aswaterman
Copy link
Contributor

D'oh. Let me look...

@aswaterman
Copy link
Contributor

Yes, but that's not the place to put it. Let me see how others do it...

@palmer-dabbelt @aswaterman
Remove the .data section from RISC-V's default linker script addon
1ffab20 
I'm actually not sure why this is here: .data isn't read-only and
shouldn't be in the INITIAL_READONLY_SECTIONS variable.  This causes a
handful of sections that should be read-only to actually be read-write
when loaded, which breaks things like "-z relro".

Removing this causes the default .data mapping to be used, which comes
from upstream.  As far os I can tell, there was no reason we were ever
supposed to have our own .data in the first place, and no other
architectures have ever done it this way.

I tested this still builds and boots Busybox on Linux.

Thanks to Alan Morda for finding the problem.

ld/ChangeLog

2016-12-29  Palmer Dabbelt <[email protected]>

        * emulparams/elf32lriscv-defs.sh (INITIAL_READONLY_SECTIONS):
        Remove the RISC-V-specific data section mappings.
@aswaterman
Copy link
Contributor

attempt 2 is pushed (fortunately it deleted yet more code). can you retry?

@palmer-dabbelt
Copy link
Contributor Author

GCC stage 2 is building, so it's probably fine.

@palmer-dabbelt palmer-dabbelt force-pushed the riscv-next branch 2 times, most recently from 30ff9e8 to 08219b2 Compare January 9, 2017 17:25
@aswaterman aswaterman force-pushed the riscv-next branch 2 times, most recently from 875eae0 to 08219b2 Compare January 20, 2017 10:04
@palmer-dabbelt
Copy link
Contributor Author

I guess it ended up building :).

@aswaterman aswaterman deleted the remove-our-data branch February 8, 2017 19:35
Nelson1225 pushed a commit to Nelson1225/riscv-binutils-gdb that referenced this pull request Jul 22, 2021
@simark
gdb: set current thread in btrace_compute_ftrace_{bts,pt}
ecd799b 
As documented in bug 28086, test gdb.btrace/enable-new-thread.exp
started failing with commit 0618ae4 ("gdb: optimize
all_matching_threads_iterator"):

    (gdb) record btrace^M
    (gdb) PASS: gdb.btrace/enable-new-thread.exp: record btrace
    break 24^M
    Breakpoint 2 at 0x555555555175: file /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.btrace/enable-new-thread.c, line 24.^M
    (gdb) continue^M
    Continuing.^M
    /home/smarchi/src/binutils-gdb/gdb/inferior.c:303: internal-error: inferior* find_inferior_pid(process_stratum_target*, int): Assertion `pid != 0' failed.^M
    A problem internal to GDB has been detected,^M
    further debugging may prove unreliable.^M
    Quit this debugging session? (y or n) FAIL: gdb.btrace/enable-new-thread.exp: continue to breakpoint: cont to bp.1 (GDB internal error)

Note that I only see the failure if GDB is compiled without libipt
support.  This is because GDB then makes use BTS instead of PT, so
exercises different code paths.

I think that the commit above just exposed an existing problem.  The
stack trace of the internal error is:

    riscvarchive#8  0x0000561cb81e404e in internal_error (file=0x561cb83aa2f8 "/home/smarchi/src/binutils-gdb/gdb/inferior.c", line=303, fmt=0x561cb83aa099 "%s: Assertion `%s' failed.") at /home/smarchi/src/binutils-gdb/gdbsupport/errors.cc:55
    riscvarchive#9  0x0000561cb7b5c031 in find_inferior_pid (targ=0x561cb8aafb60 <the_amd64_linux_nat_target>, pid=0) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:303
    riscvarchive#10 0x0000561cb7b5c102 in find_inferior_ptid (targ=0x561cb8aafb60 <the_amd64_linux_nat_target>, ptid=...) at /home/smarchi/src/binutils-gdb/gdb/inferior.c:317
    riscvarchive#11 0x0000561cb7f1d1c3 in find_thread_ptid (targ=0x561cb8aafb60 <the_amd64_linux_nat_target>, ptid=...) at /home/smarchi/src/binutils-gdb/gdb/thread.c:487
    riscvarchive#12 0x0000561cb7f1b921 in all_matching_threads_iterator::all_matching_threads_iterator (this=0x7ffc4ee34678, filter_target=0x561cb8aafb60 <the_amd64_linux_nat_target>, filter_ptid=...) at /home/smarchi/src/binutils-gdb/gdb/thread-iter.c:125
    riscvarchive#13 0x0000561cb77bc462 in filtered_iterator<all_matching_threads_iterator, non_exited_thread_filter>::filtered_iterator<process_stratum_target* const&, ptid_t const&> (this=0x7ffc4ee34670) at /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/filtered-iterator.h:42
    riscvarchive#14 0x0000561cb77b97cb in all_non_exited_threads_range::begin (this=0x7ffc4ee34650) at /home/smarchi/src/binutils-gdb/gdb/thread-iter.h:243
    riscvarchive#15 0x0000561cb7d8ba30 in record_btrace_target::record_is_replaying (this=0x561cb8aa6250 <record_btrace_ops>, ptid=...) at /home/smarchi/src/binutils-gdb/gdb/record-btrace.c:1411
    riscvarchive#16 0x0000561cb7d8bb83 in record_btrace_target::xfer_partial (this=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_MEMORY, annex=0x0, readbuf=0x7ffc4ee34c58 "\260g\343N\374\177", writebuf=0x0, offset=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/record-btrace.c:1437
    riscvarchive#17 0x0000561cb7ef73a9 in raw_memory_xfer_partial (ops=0x561cb8aa6250 <record_btrace_ops>, readbuf=0x7ffc4ee34c58 "\260g\343N\374\177", writebuf=0x0, memaddr=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/target.c:1504
    riscvarchive#18 0x0000561cb7ef77da in memory_xfer_partial_1 (ops=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_CODE_MEMORY, readbuf=0x7ffc4ee34c58 "\260g\343N\374\177", writebuf=0x0, memaddr=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/target.c:1635
    riscvarchive#19 0x0000561cb7ef78b5 in memory_xfer_partial (ops=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_CODE_MEMORY, readbuf=0x7ffc4ee34c58 "\260g\343N\374\177", writebuf=0x0, memaddr=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/target.c:1664
    riscvarchive#20 0x0000561cb7ef7ba4 in target_xfer_partial (ops=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_CODE_MEMORY, annex=0x0, readbuf=0x7ffc4ee34c58 "\260g\343N\374\177", writebuf=0x0, offset=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/target.c:1721
    riscvarchive#21 0x0000561cb7ef8503 in target_read_partial (ops=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_CODE_MEMORY, annex=0x0, buf=0x7ffc4ee34c58 "\260g\343N\374\177", offset=140737352774277, len=1, xfered_len=0x7ffc4ee34ad8) at /home/smarchi/src/binutils-gdb/gdb/target.c:1974
    riscvarchive#22 0x0000561cb7ef861f in target_read (ops=0x561cb8aa6250 <record_btrace_ops>, object=TARGET_OBJECT_CODE_MEMORY, annex=0x0, buf=0x7ffc4ee34c58 "\260g\343N\374\177", offset=140737352774277, len=1) at /home/smarchi/src/binutils-gdb/gdb/target.c:2014
    riscvarchive#23 0x0000561cb7ef809f in target_read_code (memaddr=140737352774277, myaddr=0x7ffc4ee34c58 "\260g\343N\374\177", len=1) at /home/smarchi/src/binutils-gdb/gdb/target.c:1869
    riscvarchive#24 0x0000561cb7937f4d in gdb_disassembler::dis_asm_read_memory (memaddr=140737352774277, myaddr=0x7ffc4ee34c58 "\260g\343N\374\177", len=1, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/gdb/disasm.c:139
    riscvarchive#25 0x0000561cb80ab66d in fetch_data (info=0x7ffc4ee34e88, addr=0x7ffc4ee34c59 "g\343N\374\177") at /home/smarchi/src/binutils-gdb/opcodes/i386-dis.c:194
    riscvarchive#26 0x0000561cb80ab7e2 in ckprefix () at /home/smarchi/src/binutils-gdb/opcodes/i386-dis.c:8628
    riscvarchive#27 0x0000561cb80adbd8 in print_insn (pc=140737352774277, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/opcodes/i386-dis.c:9587
    riscvarchive#28 0x0000561cb80abe4f in print_insn_i386 (pc=140737352774277, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/opcodes/i386-dis.c:8894
    riscvarchive#29 0x0000561cb7744a19 in default_print_insn (memaddr=140737352774277, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/gdb/arch-utils.c:1029
    riscvarchive#30 0x0000561cb7b33067 in i386_print_insn (pc=140737352774277, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/gdb/i386-tdep.c:4013
    riscvarchive#31 0x0000561cb7acd8f4 in gdbarch_print_insn (gdbarch=0x561cbae2fb60, vma=140737352774277, info=0x7ffc4ee34e88) at /home/smarchi/src/binutils-gdb/gdb/gdbarch.c:3478
    riscvarchive#32 0x0000561cb793a32d in gdb_disassembler::print_insn (this=0x7ffc4ee34e80, memaddr=140737352774277, branch_delay_insns=0x0) at /home/smarchi/src/binutils-gdb/gdb/disasm.c:795
    riscvarchive#33 0x0000561cb793a5b0 in gdb_print_insn (gdbarch=0x561cbae2fb60, memaddr=140737352774277, stream=0x561cb8ac99f8 <null_stream>, branch_delay_insns=0x0) at /home/smarchi/src/binutils-gdb/gdb/disasm.c:850
    riscvarchive#34 0x0000561cb793a631 in gdb_insn_length (gdbarch=0x561cbae2fb60, addr=140737352774277) at /home/smarchi/src/binutils-gdb/gdb/disasm.c:859
    riscvarchive#35 0x0000561cb77f53f4 in btrace_compute_ftrace_bts (tp=0x561cbba11210, btrace=0x7ffc4ee35188, gaps=...) at /home/smarchi/src/binutils-gdb/gdb/btrace.c:1107
    riscvarchive#36 0x0000561cb77f55f5 in btrace_compute_ftrace_1 (tp=0x561cbba11210, btrace=0x7ffc4ee35180, cpu=0x0, gaps=...) at /home/smarchi/src/binutils-gdb/gdb/btrace.c:1527
    riscvarchive#37 0x0000561cb77f5705 in btrace_compute_ftrace (tp=0x561cbba11210, btrace=0x7ffc4ee35180, cpu=0x0) at /home/smarchi/src/binutils-gdb/gdb/btrace.c:1560
    riscvarchive#38 0x0000561cb77f583b in btrace_add_pc (tp=0x561cbba11210) at /home/smarchi/src/binutils-gdb/gdb/btrace.c:1589
    riscvarchive#39 0x0000561cb77f5a86 in btrace_enable (tp=0x561cbba11210, conf=0x561cb8ac6878 <record_btrace_conf>) at /home/smarchi/src/binutils-gdb/gdb/btrace.c:1629
    riscvarchive#40 0x0000561cb7d88d26 in record_btrace_enable_warn (tp=0x561cbba11210) at /home/smarchi/src/binutils-gdb/gdb/record-btrace.c:294
    riscvarchive#41 0x0000561cb7c603dc in std::__invoke_impl<void, void (*&)(thread_info*), thread_info*> (__f=@0x561cbb6c4878: 0x561cb7d88cdc <record_btrace_enable_warn(thread_info*)>) at /usr/include/c++/10/bits/invoke.h:60
    riscvarchive#42 0x0000561cb7c5e5a6 in std::__invoke_r<void, void (*&)(thread_info*), thread_info*> (__fn=@0x561cbb6c4878: 0x561cb7d88cdc <record_btrace_enable_warn(thread_info*)>) at /usr/include/c++/10/bits/invoke.h:153
    riscvarchive#43 0x0000561cb7c5dc92 in std::_Function_handler<void (thread_info*), void (*)(thread_info*)>::_M_invoke(std::_Any_data const&, thread_info*&&) (__functor=..., __args#0=@0x7ffc4ee35310: 0x561cbba11210) at /usr/include/c++/10/bits/std_function.h:291
    riscvarchive#44 0x0000561cb7f2600f in std::function<void (thread_info*)>::operator()(thread_info*) const (this=0x561cbb6c4878, __args#0=0x561cbba11210) at /usr/include/c++/10/bits/std_function.h:622
    riscvarchive#45 0x0000561cb7f23dc8 in gdb::observers::observable<thread_info*>::notify (this=0x561cb8ac5aa0 <gdb::observers::new_thread>, args#0=0x561cbba11210) at /home/smarchi/src/binutils-gdb/gdb/../gdbsupport/observable.h:150
    riscvarchive#46 0x0000561cb7f1c436 in add_thread_silent (targ=0x561cb8aafb60 <the_amd64_linux_nat_target>, ptid=...) at /home/smarchi/src/binutils-gdb/gdb/thread.c:263
    riscvarchive#47 0x0000561cb7f1c479 in add_thread_with_info (targ=0x561cb8aafb60 <the_amd64_linux_nat_target>, ptid=..., priv=0x561cbb3f7ab0) at /home/smarchi/src/binutils-gdb/gdb/thread.c:272
    riscvarchive#48 0x0000561cb7bfa1d0 in record_thread (info=0x561cbb0413a0, tp=0x0, ptid=..., th_p=0x7ffc4ee35610, ti_p=0x7ffc4ee35620) at /home/smarchi/src/binutils-gdb/gdb/linux-thread-db.c:1380
    riscvarchive#49 0x0000561cb7bf7a2a in thread_from_lwp (stopped=0x561cba81db20, ptid=...) at /home/smarchi/src/binutils-gdb/gdb/linux-thread-db.c:429
    riscvarchive#50 0x0000561cb7bf7ac5 in thread_db_notice_clone (parent=..., child=...) at /home/smarchi/src/binutils-gdb/gdb/linux-thread-db.c:447
    riscvarchive#51 0x0000561cb7bdc9a2 in linux_handle_extended_wait (lp=0x561cbae25720, status=4991) at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:1981
    riscvarchive#52 0x0000561cb7bdf0f3 in linux_nat_filter_event (lwpid=435403, status=198015) at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:2920
    riscvarchive#53 0x0000561cb7bdfed6 in linux_nat_wait_1 (ptid=..., ourstatus=0x7ffc4ee36398, target_options=...) at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:3202
    riscvarchive#54 0x0000561cb7be0b68 in linux_nat_target::wait (this=0x561cb8aafb60 <the_amd64_linux_nat_target>, ptid=..., ourstatus=0x7ffc4ee36398, target_options=...) at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:3440
    riscvarchive#55 0x0000561cb7bfa2fc in thread_db_target::wait (this=0x561cb8a9acd0 <the_thread_db_target>, ptid=..., ourstatus=0x7ffc4ee36398, options=...) at /home/smarchi/src/binutils-gdb/gdb/linux-thread-db.c:1412
    riscvarchive#56 0x0000561cb7d8e356 in record_btrace_target::wait (this=0x561cb8aa6250 <record_btrace_ops>, ptid=..., status=0x7ffc4ee36398, options=...) at /home/smarchi/src/binutils-gdb/gdb/record-btrace.c:2547
    riscvarchive#57 0x0000561cb7ef996d in target_wait (ptid=..., status=0x7ffc4ee36398, options=...) at /home/smarchi/src/binutils-gdb/gdb/target.c:2608
    riscvarchive#58 0x0000561cb7b6d297 in do_target_wait_1 (inf=0x561cba6d8780, ptid=..., status=0x7ffc4ee36398, options=...) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:3640
    riscvarchive#59 0x0000561cb7b6d43e in operator() (__closure=0x7ffc4ee36190, inf=0x561cba6d8780) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:3701
    riscvarchive#60 0x0000561cb7b6d7b2 in do_target_wait (ecs=0x7ffc4ee36370, options=...) at /home/smarchi/src/binutils-gdb/gdb/infrun.c:3720
    riscvarchive#61 0x0000561cb7b6e67d in fetch_inferior_event () at /home/smarchi/src/binutils-gdb/gdb/infrun.c:4069
    riscvarchive#62 0x0000561cb7b4659b in inferior_event_handler (event_type=INF_REG_EVENT) at /home/smarchi/src/binutils-gdb/gdb/inf-loop.c:41
    riscvarchive#63 0x0000561cb7be25f7 in handle_target_event (error=0, client_data=0x0) at /home/smarchi/src/binutils-gdb/gdb/linux-nat.c:4227
    riscvarchive#64 0x0000561cb81e4ee2 in handle_file_event (file_ptr=0x561cbae24e10, ready_mask=1) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:575
    riscvarchive#65 0x0000561cb81e5490 in gdb_wait_for_event (block=0) at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:701
    riscvarchive#66 0x0000561cb81e41be in gdb_do_one_event () at /home/smarchi/src/binutils-gdb/gdbsupport/event-loop.cc:212
    riscvarchive#67 0x0000561cb7c18096 in start_event_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:421
    riscvarchive#68 0x0000561cb7c181e0 in captured_command_loop () at /home/smarchi/src/binutils-gdb/gdb/main.c:481
    riscvarchive#69 0x0000561cb7c19d7e in captured_main (data=0x7ffc4ee366a0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1353
    riscvarchive#70 0x0000561cb7c19df0 in gdb_main (args=0x7ffc4ee366a0) at /home/smarchi/src/binutils-gdb/gdb/main.c:1368
    riscvarchive#71 0x0000561cb7693186 in main (argc=11, argv=0x7ffc4ee367b8) at /home/smarchi/src/binutils-gdb/gdb/gdb.c:32

At frame 45, the new_thread observable is fired.  At this moment, the
new thread isn't the current thread, inferior_ptid is null_ptid.  I
think this is ok: the new_thread observable doesn't give any guarantee
on the global context when observers are invoked.  Frame 35,
btrace_compute_ftrace_bts, calls gdb_insn_length.  gdb_insn_length
doesn't have a thread_info or other parameter what could indicate where
to read memory from, it implicitly uses the global context
(inferior_ptid).

So we reach the all_non_exited_threads_range in
record_btrace_target::record_is_replaying with a null inferior_ptid.
The previous implemention of all_non_exited_threads_range didn't care,
but the new one does.  The problem of calling gdb_insn_length and
ultimately trying to read memory with a null inferior_ptid already
existed, but the commit mentioned above made it visible.

Something between frames 40 (record_btrace_enable_warn) and 35
(btrace_compute_ftrace_bts) needs to be switching the global context to
make TP the current thread.  Since btrace_compute_ftrace_bts takes the
thread_info to work with as a parameter, that typically means that it
doesn't require its caller to also set the global current context
(current thread) when calling.  If it needs to call other functions
that do require the global current thread to be set, then it needs to
temporarily change the current thread while calling these other
functions.  Therefore, switch and restore the current thread in
btrace_compute_ftrace_bts.

By inspection, it looks like btrace_compute_ftrace_pt may also call
functions sensitive to the global context: it installs the
btrace_pt_readmem_callback callback in the PT instruction decoder.  When
this function gets called, inferior_ptid must be set appropriately.  Add
a switch and restore in there too.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28086
Change-Id: I407fbfe41aab990068bd102491aa3709b0a034b3
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kito-cheng kito-cheng kito-cheng left review comments

Assignees

@palmer-dabbelt palmer-dabbelt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@palmer-dabbelt @aswaterman @kito-cheng