Bug 1975895 - glibc: Preserve symtab in libc.so.6 and other critical shared objects
Summary: glibc: Preserve symtab in libc.so.6 and other critical shared objects
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: glibc
Version: 33
Hardware: Unspecified
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Florian Weimer
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2021-06-24 16:32 UTC by Florian Weimer
Modified: 2021-07-20 01:09 UTC (History)
14 users (show)

Fixed In Version: glibc-2.33.9000-31.fc35 glibc-2.33-19.fc34 glibc-2.32-10.fc33
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-07-20 01:09:43 UTC
Type: Bug


Attachments (Terms of Use)
LLDB workaround of buggy glibc-2.33-18.fc34.x86_64 (794 bytes, patch)
2021-06-26 20:40 UTC, Jan Kratochvil
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Bugzilla 1974357 1 unspecified CLOSED glibc pthreads updates break helgrind 2021-12-07 21:34:10 UTC

Internal Links: 1974357

Description Florian Weimer 2021-06-24 16:32:46 UTC
This is a continuation of bug 1973026 and bug 1965374. It seems that perf (the kernel tool) cannot merge .dynsym and .symtab like other tools, so it needs a fairly complete symbol table to be operational.

Comment 1 Florian Weimer 2021-06-24 16:35:34 UTC
Also see bug 1974357 comment 9.

Comment 2 Jan Kratochvil 2021-06-26 20:40:14 UTC
Created attachment 1794878 [details]
LLDB workaround of buggy glibc-2.33-18.fc34.x86_64

FAIL: glibc-2.33-18.fc34.x86_64
FAIL: glibc-2.33.9000-25.fc35.x86_64

This change broke LLDB:
Failed Tests (17):
  lldb-api :: commands/expression/import-std-module/array/TestArrayFromStdModule.py
  lldb-api :: commands/expression/import-std-module/basic/TestImportStdModule.py
  lldb-api :: commands/expression/import-std-module/conflicts/TestStdModuleWithConflicts.py
  lldb-api :: commands/expression/import-std-module/queue/TestQueueFromStdModule.py
  lldb-api :: commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  lldb-api :: commands/expression/import-std-module/vector/TestVectorFromStdModule.py
  lldb-api :: commands/expression/radar_9531204/TestPrintfAfterUp.py
  lldb-api :: commands/expression/save_jit_objects/TestSaveJITObjects.py
  lldb-api :: commands/expression/test/TestExprs.py
  lldb-api :: functionalities/process_group/TestChangeProcessGroup.py
  lldb-api :: functionalities/thread/state_after_expression/TestStateAfterExpression.py
  lldb-api :: functionalities/unwind/noreturn/TestNoreturnUnwind.py
  lldb-api :: lang/c/function_types/TestFunctionTypes.py
  lldb-api :: lang/c/stepping/TestStepAndBreakpoints.py
  lldb-api :: lang/cpp/printf/TestPrintf.py
  lldb-shell :: Recognizer/assert.test
  lldb-shell :: ScriptInterpreter/Lua/nested_sessions.test

Currently there is:
  /lib64/libc.so.6 .dynsym = sufficient symbol table (2419 entries)
  /lib64/libc.so.6 .symtab = IMO useless few symbols (104 entries).
  /usr/lib/debug/lib64/libc-2.33.so-2.33-18.fc34.x86_64.debug
    = big symbol table (28725 entries mostly useless .annobin_lto_* ones)

I would like to delete (which my LLDB patch does):
  /lib64/libc.so.6 .symtab

Some consumers will choose only the first table they find in this order:
  .symtab in /lib64/libc.so.6
  .symtab in /usr/lib/debug/lib64/libc*.debug
  .dynsym in /lib64/libc.so.6
  .dynsym in /usr/lib/debug/lib64/libc*.debug

I find that correct as that is what https://refspecs.linuxbase.org/elf/elf.pdf says:
  SHT_SYMTAB provides [...].  As a complete symbol table [...]
  may also contain a SHT_DYNSYM section,
  which holds a minimal set of dynamic linking symbols, to save space

I read that as .symtab is a superset of .dynsym, if present.
That is not the case of current glibc.

Comment 3 Jan Kratochvil 2021-06-26 20:43:08 UTC
(In reply to Jan Kratochvil from comment #2)
> Some consumers will choose only the first table they find in this order:
>   .symtab in /lib64/libc.so.6
>   .symtab in /usr/lib/debug/lib64/libc*.debug
>   .dynsym in /lib64/libc.so.6
>   .dynsym in /usr/lib/debug/lib64/libc*.debug

In fact LLDB uses this order:
  .symtab in /usr/lib/debug/lib64/libc*.debug
  .symtab in /lib64/libc.so.6
  .dynsym in /usr/lib/debug/lib64/libc*.debug
  .dynsym in /lib64/libc.so.6

But that does not matter when *-debuginfo.rpm is not installed (or it is intentionally disabled as is for the LLDB testsuite).

Comment 4 Florian Weimer 2021-06-27 08:53:54 UTC
(In reply to Jan Kratochvil from comment #2)
> Currently there is:
>   /lib64/libc.so.6 .dynsym = sufficient symbol table (2419 entries)
>   /lib64/libc.so.6 .symtab = IMO useless few symbols (104 entries).
>   /usr/lib/debug/lib64/libc-2.33.so-2.33-18.fc34.x86_64.debug
>     = big symbol table (28725 entries mostly useless .annobin_lto_* ones)
> 
> I would like to delete (which my LLDB patch does):
>   /lib64/libc.so.6 .symtab

A minimal .symtab is required by valgrind. We cannot make valgrind (or other tools) depend on glibc-debuginfo because that is not available in Koji buildroots. (A debuginfo dependency is how Debian deals with this issue.)

We can add more symbols to .symtab if other tools need them (but we should probably filter out cruft like the .annobin_lto_* symbols).

Comment 5 Jan Kratochvil 2021-06-27 09:24:51 UTC
(In reply to Florian Weimer from comment #4)
> A minimal .symtab is required by valgrind. We cannot make valgrind (or other
> tools) depend on glibc-debuginfo because that is not available in Koji
> buildroots. (A debuginfo dependency is how Debian deals with this issue.)

Fedora valgrind should be able to read all such symbols from .gnu_debugdata:
  https://fedoraproject.org/wiki/Features/MiniDebugInfo
Which is missing in the current glibc build, that could be filed as another glibc Bug.

Then there is no longer a need to have two different .symtab (in main vs. *.debug binary).

Comment 6 Florian Weimer 2021-06-27 09:45:58 UTC
(In reply to Jan Kratochvil from comment #5)
> (In reply to Florian Weimer from comment #4)
> > A minimal .symtab is required by valgrind. We cannot make valgrind (or other
> > tools) depend on glibc-debuginfo because that is not available in Koji
> > buildroots. (A debuginfo dependency is how Debian deals with this issue.)
> 
> Fedora valgrind should be able to read all such symbols from .gnu_debugdata:
>   https://fedoraproject.org/wiki/Features/MiniDebugInfo
> Which is missing in the current glibc build, that could be filed as another
> glibc Bug.

glibc-2.33-8.fc34 had .gnu_debugdata, but valgrind did not like it, which is why we went down the .symtab route.

However, the symbols in .gnu_debugdata require merging with .dynsym because .dynsym are missing from .gnu_debugdata. Quoting find-debuginfo.sh:

  # Extract the dynamic symbols from the main binary, there is no need to also have these
  # in the normal symbol table
  nm -D "$binary" --format=posix --defined-only | awk '{ print $1 }' | sort > "$dynsyms"
  # Extract all the text (i.e. function) symbols from the debuginfo
  # Use format sysv to make sure we can match against the actual ELF FUNC
  # symbol type. The binutils nm posix format symbol type chars are
  # ambigous for architectures that might use function descriptors.
  nm "$debuginfo" --format=sysv --defined-only | awk -F \| '{ if ($4 ~ "FUNC") print $1 }' | sort > "$funcsyms"
  # Keep all the function symbols not already in the dynamic symbol table
  comm -13 "$dynsyms" "$funcsyms" > "$keep_symbols"

If LLDB supports .gnu_debugdata, it already supports some form of symbol table merging, so why not extend that to .symtab?

And we should teach more tools to process .gnu_debugdata.

Comment 7 Jan Kratochvil 2021-06-27 10:36:32 UTC
(In reply to Florian Weimer from comment #6)
> glibc-2.33-8.fc34 had .gnu_debugdata, but valgrind did not like it, which is
> why we went down the .symtab route.

That is a bug of Valgrind which should be fixed in Valgrind. If valgrind matters at all which I do not see why it would:
  https://developers.redhat.com/blog/2021/05/05/memory-error-checking-in-c-and-c-comparing-sanitizers-and-valgrind


> However, the symbols in .gnu_debugdata require merging with .dynsym because
> .dynsym are missing from .gnu_debugdata.

Yes but that is .gnu_debugdata vs. (.dynsym|.symtab), that is done in LLDB.
Not for .dynsym vs. .symtab because that has no real reason to do so.


> If LLDB supports .gnu_debugdata, it already supports some form of symbol table merging,

Yes, it does (written by Konrad Kleine).


> so why not extend that to .symtab?

Because that would be only a workaround of ELF workaround of a Valgrind bug. I cannot imagine how to justify that upstream.

Comment 8 Fedora Update System 2021-06-28 09:37:29 UTC
FEDORA-2021-abc33e227a has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 9 Florian Weimer 2021-06-28 09:38:48 UTC
Jan, do the changes now in rawhide (via glibc-2.33.9000-31.fc35) help LLDB? Thanks.

Comment 10 Jan Kratochvil 2021-06-28 12:10:25 UTC
It is no longer "Failed Tests (17)" but it is still:
Failed Tests (3):
  lldb-api :: functionalities/unwind/noreturn/TestNoreturnUnwind.py
  lldb-shell :: Recognizer/assert.test
  lldb-shell :: ScriptInterpreter/Lua/nested_sessions.test

(in both cases the /Lua/ test is a false positive so it is 16 vs. 2 failed tests)

FAIL: glibc-2.33.9000-31.fc35.x86_64
* thread #1, name = '1', stop reason = signal SIGABRT
    frame #0: 0x00007ffff7e48ee3 libc.so.6`pthread_kill@GLIBC_2.2.5 + 67
  * frame #1: 0x00007ffff7dfb986 libc.so.6`raise + 22
    frame #2: 0x00007ffff7de5806 libc.so.6`abort + 230
    frame #3: 0x00007ffff7de571b libc.so.6`__assert_fail_base.cold + 15
    frame #4: 0x00007ffff7df4646 libc.so.6`__assert_fail + 70
    frame #5: 0x00000000004011bd 1`main at assert.c:7:3
    frame #6: 0x00007ffff7de67e0 libc.so.6`__libc_start_call_main + 128
    frame #7: 0x00007ffff7de688c libc.so.6`__libc_start_main@@GLIBC_2.34 + 124
    frame #8: 0x0000000000401065 1`_start + 37

PASS: glibc-2.33-14.fc34.x86_64
* thread #1, name = '1', stop reason = hit program assert
    frame #0: 0x00007ffff7e242a2 libc.so.6`raise + 322
    frame #1: 0x00007ffff7e0d8a4 libc.so.6`abort + 278
    frame #2: 0x00007ffff7e0d789 libc.so.6`__assert_fail_base.cold + 15
    frame #3: 0x00007ffff7e1ca16 libc.so.6`__assert_fail + 70
  * frame #4: 0x00000000004011bd 1`main at assert.c:7:3
    frame #5: 0x00007ffff7e0eb75 libc.so.6`__libc_start_main + 213
    frame #6: 0x000000000040106e 1`_start + 46

I have to investigate it more why it still fails, I agree .symtab now looks OK to me.

Comment 11 Florian Weimer 2021-06-28 12:13:45 UTC
Thanks for checking.  My guess is that LLDB's assert failure detection does not like the additional pthread_kill frame.

Comment 12 Jan Kratochvil 2021-06-29 17:14:10 UTC
(In reply to Florian Weimer from comment #11)
> My guess is that LLDB's assert failure detection does
> not like the additional pthread_kill frame.

This obvious guess is right. I have submitted it upstream:
  https://reviews.llvm.org/D105133

Comment 13 Carlos O'Donell 2021-07-06 13:24:26 UTC
This still needs to be reviewed for inclusion into F33 and F34.

Comment 14 Fedora Update System 2021-07-13 14:02:40 UTC
FEDORA-2021-3f4132bb56 has been submitted as an update to Fedora 33. https://bodhi.fedoraproject.org/updates/FEDORA-2021-3f4132bb56

Comment 15 Fedora Update System 2021-07-14 01:09:13 UTC
FEDORA-2021-3f4132bb56 has been pushed to the Fedora 33 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-3f4132bb56`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-3f4132bb56

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 16 Fedora Update System 2021-07-20 01:09:43 UTC
FEDORA-2021-3f4132bb56 has been pushed to the Fedora 33 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.