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

Check the upper bits of e_flags #49

Closed
palmer-dabbelt opened this issue Jan 2, 2017 · 5 comments
Closed

Check the upper bits of e_flags #49

palmer-dabbelt opened this issue Jan 2, 2017 · 5 comments

Comments

@palmer-dabbelt
Copy link
Contributor

No description provided.

@palmer-dabbelt
Copy link
Contributor Author

#58

@aswaterman
Copy link
Contributor

One thing though, it's possible that some flags will get introduced later that don't imply ABI incompatibility. I think the main place this will show up is linking against a newer libX.a / libX.so using an older linker. (The dynamic linker doesn't check e_flags at the moment, but it probably should, so this would also be a problem with using newer executables with an older dynamic linker.)

By going forward with this, we're basically saying that each e_flags setting represents its own, incompatible ABI. One thing we could consider doing is reserving the lower half-word for ABI, and the upper half-word for other stuff that doesn't imply incompatibility.

Probably best to research what other BFD ports do.

@palmer-dabbelt
Copy link
Contributor Author

One thing I was thinking of doing was having a "equal flags are compatible" check, which is in ARMv8

  /* Identical flags must be compatible.  */
  if (in_flags == out_flags)
    return TRUE;

but just for the unknown bits, so the patch would look more like

+  if ((new_flags & EF_RISCV_RESERVED) != (old_flags & EF_RISCV_RESERVED))
 +    {
 +      (*_bfd_error_handler)
 +        (_("%B: Unknown ELF flag `%x'"),
 +	 ibfd, new_flags);
 +      return FALSE;
 +    }

but I decided not to. My reasoning had something to do with a version field, but that doesn't make any sense (so I was either too hungry or am now too full).

I don't see any EF_ in aarch64, and this http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056c/IHI0056C_beta_aaelf64.pdf says "There are no processor-specific flags so [e_flags] shall contain zero." so this might not be relevant.

It looks like MIPS ran into a similar issue. I see

  /* Some IRIX 6 BSD-compatibility objects have this bit set.  It
     doesn't seem to matter.  */
  new_flags &= ~EF_MIPS_XGOT;
  old_flags &= ~EF_MIPS_XGOT;

the rest of the MIPS e_flags stuff is a mess, but I don't see them checking reserved bits anywhere.

Tilera doesn't appear to define any EF_ flags, they just check the target names explicitly. IIRC they never had any ISA subsets.

SPARC has some EF_ stuff, but it doesn't check them when merging.

The only reference to e_flags I can find anywhere in glibc is in ARM

      if (!ret && EF_ARM_EABI_VERSION (elf32_header->e_flags) == EF_ARM_EABI_VER5)
        {
          if (elf32_header->e_flags & EF_ARM_ABI_FLOAT_HARD)
            *flag = FLAG_ARM_LIBHF|FLAG_ELF_LIBC6;
          else if (elf32_header->e_flags & EF_ARM_ABI_FLOAT_SOFT)
            *flag = FLAG_ARM_LIBSF|FLAG_ELF_LIBC6;
          else
            /* We must assume the unmarked objects are compatible
               with all ABI variants. Such objects may have been
               generated in a transitional period when the ABI
               tags were not added to all objects.  */
            *flag = FLAG_ELF_LIBC6;
        }

which makes me think that maybe it's just not worth checking the reserved bits at all.

@aswaterman
Copy link
Contributor

Yeah, I think not checking them at all is the better thing to do.

The psABI spec is the line of defense here. If people allocate reserved flags for their own purposes, then they are knowingly making an incompatibility. If future versions of binutils start to reject their libraries, it's their fault.

@palmer-dabbelt
Copy link
Contributor Author

Works for me. I'm closing this and the PR.

Nelson1225 pushed a commit to Nelson1225/riscv-binutils-gdb that referenced this issue Jul 22, 2021
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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants