Skip to content

fix(debuginfo): name size-0 asm labels instead of raw addresses #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
not-matthias wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(debuginfo): name size-0 asm labels instead of raw addresses #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1789,6 +1789,7 @@
/none/tests/amd64/smc1
/none/tests/amd64/sse4-64
/none/tests/amd64/ssse3_misaligned
/none/tests/amd64/synth_label
/none/tests/amd64/tm1
/none/tests/amd64/x87trigOOR
/none/tests/amd64/xacq_xrel
Expand Down
56 changes: 40 additions & 16 deletions coregrind/m_debuginfo/readelf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,22 +285,16 @@ Bool get_elf_symbol_info (
)
{
Bool plausible;
Bool is_size0_label = False;
# if defined(VGP_ppc64be_linux)
Bool is_in_opd;
# endif
Bool in_text, in_data, in_sdata, in_rodata, in_bss, in_sbss;
Addr text_svma, data_svma, sdata_svma, rodata_svma, bss_svma, sbss_svma;
PtrdiffT text_bias, data_bias, sdata_bias, rodata_bias, bss_bias, sbss_bias;
# if defined(VGPV_arm_linux_android) \
|| defined(VGPV_x86_linux_android) \
|| defined(VGPV_mips32_linux_android) \
|| defined(VGPV_arm64_linux_android)
Addr available_size = 0;
#define COMPUTE_AVAILABLE_SIZE(segsvma, segsize) \
available_size = segsvma + segsize - sym_svma
#else
#define COMPUTE_AVAILABLE_SIZE(segsvma, segsize)
#endif

/* Set defaults */
*sym_name_out_ioff = sym_name_ioff;
Expand Down Expand Up @@ -454,6 +448,25 @@ Bool get_elf_symbol_info (
plausible = True;
# endif

/* Handwritten assembly often lacks .size directives, leaving entry
points as size-0 STT_NOTYPE labels (e.g. glibc's _dl_start_user).
Admit them when they lie in a known text section (available_size
is only nonzero when a section range check above matched); the
size stays 0 and canonicaliseSymtab later extends the label up to
the next symbol's start, as perf's symbols__fixup_end does.
'$'-prefixed names are ARM/AArch64 mapping symbols, not code
entry points. */
if (!plausible
&& *is_text_out
&& ELFXX_ST_TYPE(sym->st_info) == STT_NOTYPE
&& *sym_size_out == 0
&& available_size > 0
&& sym_name_ioff != DiOffT_INVALID
&& ML_(img_get_UChar)(escn_strtab->img, sym_name_ioff) != '$') {
plausible = True;
is_size0_label = True;
}

if (!plausible)
return False;

Expand Down Expand Up @@ -491,13 +504,17 @@ Bool get_elf_symbol_info (
|| defined(VGPV_arm64_linux_android)
*sym_size_out = available_size ? available_size : 2048;
# else
if (TRACE_SYMTAB_ENABLED) {
HChar* sym_name = ML_(img_strdup)(escn_strtab->img,
"di.gesi.2", sym_name_ioff);
TRACE_SYMTAB(" ignore -- size=0: %s\n", sym_name);
if (sym_name) ML_(dinfo_free)(sym_name);
if (!is_size0_label) {
if (TRACE_SYMTAB_ENABLED) {
HChar* sym_name = ML_(img_strdup)(escn_strtab->img,
"di.gesi.2", sym_name_ioff);
TRACE_SYMTAB(" ignore -- size=0: %s\n", sym_name);
if (sym_name) ML_(dinfo_free)(sym_name);
}
return False;
}
return False;
/* Keep the size 0; canonicaliseSymtab extends the label up to
the next symbol's start. */
# endif
}

Expand Down Expand Up @@ -666,10 +683,17 @@ Bool get_elf_symbol_info (
/* If no part of the symbol falls within the mapped range,
ignore it. */

/* Treat size-0 labels as occupying one byte in the text checks
below, so they cannot form a backwards range for
ML_(find_rx_mapping) (which asserts lo <= hi). Only text
labels can still be size 0 at this point, so the non-text
checks are unaffected. */
Word sym_extent = *sym_size_out > 0 ? *sym_size_out : 1;

in_text
= di->text_present
&& di->text_size > 0
&& !((*sym_avmas_out).main + *sym_size_out <= di->text_avma
&& !((*sym_avmas_out).main + sym_extent <= di->text_avma
|| (*sym_avmas_out).main >= di->text_avma + di->text_size);

in_data
Expand Down Expand Up @@ -718,13 +742,13 @@ Bool get_elf_symbol_info (
in_rx = (ML_(find_rx_mapping)(
di,
(*sym_avmas_out).main,
(*sym_avmas_out).main + *sym_size_out - 1) != NULL);
(*sym_avmas_out).main + sym_extent - 1) != NULL);
if (in_text)
vg_assert(in_rx);
if (!in_rx) {
TRACE_SYMTAB(
"ignore -- %#lx .. %#lx outside .text svma range %#lx .. %#lx\n",
(*sym_avmas_out).main, (*sym_avmas_out).main + *sym_size_out - 1,
(*sym_avmas_out).main, (*sym_avmas_out).main + sym_extent - 1,
di->text_avma,
di->text_avma + di->text_size - 1);
return False;
Expand Down
40 changes: 38 additions & 2 deletions coregrind/m_debuginfo/storage.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,8 +366,10 @@ void ML_(addSym) ( struct _DebugInfo* di, DiSym* sym )
vg_assert(sym->pri_name != NULL);
vg_assert(sym->sec_names == NULL);

/* Ignore zero-sized syms. */
if (sym->size == 0) return;
/* Ignore zero-sized symbols, except text ones: those may be
handwritten-asm labels lacking a .size directive, which
canonicaliseSymtab extends up to the next symbol's start. */
if (sym->size == 0 && !sym->isText) return;

if (di->symtab_used == di->symtab_size) {
new_sz = 2 * di->symtab_size;
Expand Down Expand Up @@ -1501,6 +1503,11 @@ static Int compare_DiSym ( const void* va, const void* vb )
const DiSym* b = vb;
if (a->avmas.main < b->avmas.main) return -1;
if (a->avmas.main > b->avmas.main) return 1;
/* Smaller size first at equal addresses: a zero-sized label then
sees the real symbol at the same address as its "next symbol" in
canonicaliseSymtab's size fixup, gets a zero gap, and is dropped,
so real symbols always win their own address. */
if (a->size != b->size) return a->size < b->size ? -1 : 1;
return 0;
}

Expand Down Expand Up @@ -1754,6 +1761,35 @@ static void canonicaliseSymtab ( struct _DebugInfo* di )
VG_(ssort)(di->symtab, di->symtab_used,
sizeof(*di->symtab), compare_DiSym);

/* Zero-sized text symbols are labels from handwritten assembly
lacking .size directives (e.g. glibc's _dl_start_user). Give
each the gap up to the next symbol's start, clamped to the end
of .text, the same way perf's symbols__fixup_end does. Anything
still zero-sized afterwards (a label aliasing a real symbol's
address, or lying outside .text) is dropped. */
{ Word r, w = 0;
for (r = 0; r < (Word)di->symtab_used; r++) {
DiSym* sym = &di->symtab[r];
if (sym->size == 0 && sym->isText && di->text_present) {
Addr end = di->text_avma + di->text_size;
if (r+1 < (Word)di->symtab_used
&& di->symtab[r+1].avmas.main < end)
end = di->symtab[r+1].avmas.main;
if (end > sym->avmas.main) {
Addr gap = end - sym->avmas.main;
Addr max_size = (1LL << 31) - 1;
sym->size = (UInt)(gap > max_size ? max_size : gap);
}
}
if (sym->size == 0)
continue;
if (w < r)
di->symtab[w] = *sym;
w++;
}
di->symtab_used = w;
}

cleanup_more:

/* BEGIN Detect and "fix" identical address ranges. */
Expand Down
Loading