New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 884234 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Oct 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Build-Toolchain



Sign in to add a comment

chrome failed to link with llvm-next

Project Member Reported by yunlian@chromium.org, Sep 14

Issue description

With lld-next
emerge-samus chromeos-chrome

python ../../../../../chromeos-cache/distfiles/target/chrome-src-internal/src/build/gn_run_binary.py host/genversion host/gen/third_party/yasm/version.mac
host/genversion failed with exit code -11

 
Cc: vapier@chromium.org
If I run this command line alone, I could not reproduce it.
By checking the environment variable in the building process, I found that it
has LD_PRELOAD=libsandbox.so in it. I can then reproduce it with 

LD_PRELOAD=libsandbox.so python ../../../../../chromeos-cache/distfiles/target/chrome-src-internal/src/build/gn_run_binary.py host/genversion host/gen/third_party/yasm/version.mac
Interestingly, the crash happens in python,

LD_PRELOAD=libsandbox.so  host/genversion host/gen/third_party/yasm/version.mac
works find.

I get the core dump and the backtrace looks like

Core was generated by `/usr/bin/python2.7 ../../../../../chromeos-cache/distfiles/target/chrome-src-in'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007fe6c1a657f7 in sb_check_exec () from /usr/lib64/libsandbox.so
(gdb) bt
#0  0x00007fe6c1a657f7 in sb_check_exec () from /usr/lib64/libsandbox.so
#1  0x00007fe6c1a650e0 in execv_DEFAULT () from /usr/lib64/libsandbox.so
#2  0x00007fe6c17a86b0 in posix_execv () from /usr/lib64/libpython2.7.so.1.0
#3  0x00007fe6c176bb29 in PyEval_EvalFrameEx () from /usr/lib64/libpython2.7.so.1.0
#4  0x00007fe6c17687e6 in PyEval_EvalCodeEx () from /usr/lib64/libpython2.7.so.1.0
#5  0x00007fe6c176f738 in fast_function () from /usr/lib64/libpython2.7.so.1.0



could experiment with newer versions of sandbox

normally you shouldn't be exporting just LD_PRELOAD.  try running `sandbox` instead to initialize the environment.  then run whatever commands you want.
upstream sandbox 2.13 still has the same error.
Running sandbox does not help either.

The crash happens in
__wrapper_exec.c 

in the MARCRO
PARSE_ELF


                                char *symname = (void *)(elf + stroff + sym->st_name); \
                                if (ELF##n##_ST_VISIBILITY(sym->st_other) == STV_DEFAULT && \
                                    sym->st_shndx != SHN_UNDEF && sym->st_shndx < SHN_LORESERVE && \
                                    sym->st_name && \
                                    /* Minor optimization to avoid strcmp. */ \
                                    symname[0] == '_' && symname[1] == '_') { \


i expect that we're hitting the same issue as we've seen elsewhere ... the ELF spec does not declare the max # of symbols anywhere in the hash table, so sandbox has a bit of a hack to try and workaround that

if we implement gnu.hash support in there like glibc does, it probably would be more robust
I tried to dump the contents of the *symname,
It shows some symbols like
_ZdlPvSt11align_val_tRKSt9nothrow_t
__libc_csu_init

In the end, it shows some non unicode characters.
Is this the issue you are referring to?
yes, it probably walked past the end of the symbol table and into garbage at which point it crashed
I compared the contents of symname bewteen a good (current lld)  and bad (llvm-next lld), it looks like that we did not walk past the end of symbol table, because the good one has a longer output of symbol table.
i suspect it's not a bug in lld.  we know the sandbox code has edge cases where it will fail (because of ELF limitations).

the right answer imo is to add support for gnu.hash walking.  if you look at the sandbox code you quoted (libsandbox/wrapper-funcs/__wrapper_exec.c), there's a great big comment block above it explaining things in detail.
When linking with gold, .strtab comes after .symtab
When linking with lld,  .shstrtab comes after .symtab

So the assumption that the .strtab alwary comes after .symtab does not hold
when liking with LLD. I will check the contents of symname when linking with gold.
I have the following patch to workaround this issue
It adds a check to make sure the symbol table index locates inside the string
table. If it goes out of bound, then it just exits.
Does this make sense?

--- __wrapper_exec.c
+++ __wrapper_exec.c
@@ -147,6 +147,8 @@
 			sym = (void *)(elf + symoff); \
 			symend = vhash ? (sym + hashes[1]) : (void *)(elf + stroff); \
 			while (sym < symend) { \
+				if (sym->st_name >= str_size) \
+				  break; \
 				char *symname = (void *)(elf + stroff + sym->st_name); \
 				if (ELF##n##_ST_VISIBILITY(sym->st_other) == STV_DEFAULT && \
 				    sym->st_shndx != SHN_UNDEF && sym->st_shndx < SHN_LORESERVE && \
Status: Verified (was: Untriaged)

Sign in to add a comment