[Lldb-commits] [PATCH] D74252: Fix+re-enable Assert StackFrame Recognizer

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added reviewers: mib, JDevlieghere, labath.
jankratochvil added a project: LLDB.
Herald added a subscriber: aprantl.

D73303 <https://reviews.llvm.org/D73303> was failing on Fedora Linux <https://reviews.llvm.org/D73303#1862486> and so it was disabled by Skip the AssertFrameRecognizer test for Linux <https://github.com/llvm/llvm-project/commit/2e005c64f3019aada8df29a24dcfe56f044e2e59>.
On Fedora 30 x86_64 I have:

  $ readelf -Ws /lib64/libc.so.6 |grep '^Symbol\|.*assert_fail'
  Symbol table '.dynsym' contains 2362 entries:
     630: 0000000000030520    70 FUNC    GLOBAL DEFAULT   14 __assert_fail@@GLIBC_2.2.5
  Symbol table '.symtab' contains 22711 entries:
     922: 000000000002275a    15 FUNC    LOCAL  DEFAULT   14 __assert_fail_base.cold
   18044: 0000000000030520    70 FUNC    LOCAL  DEFAULT   14 __GI___assert_fail
   20081: 00000000000303a0   370 FUNC    LOCAL  DEFAULT   14 __assert_fail_base
   21766: 0000000000030520    70 FUNC    GLOBAL DEFAULT   14 __assert_fail

I do not see why your patch was ever expecting `__GI___assert_fail`:
`.symtab` can be present or not but that should not change that `__assert_fail` always wins - it is always present from `.dynsym` and it can never be overriden by `__GI___assert_fail` as `__GI___assert_fail` has only local binding. Global binding is preferred since D63540 <https://reviews.llvm.org/D63540>.
External debug info symbols do not matter since D55859 <https://reviews.llvm.org/D55859> (and DWARF should never be embedded in system `libc.so.6`).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74252

Files:
  lldb/source/Target/AssertFrameRecognizer.cpp
  lldb/test/Shell/Recognizer/assert.test


Index: lldb/test/Shell/Recognizer/assert.test
===================================================================
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -1,4 +1,4 @@
-# UNSUPPORTED: system-windows, system-linux
+# UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
 run
Index: lldb/source/Target/AssertFrameRecognizer.cpp
===================================================================
--- lldb/source/Target/AssertFrameRecognizer.cpp
+++ lldb/source/Target/AssertFrameRecognizer.cpp
@@ -98,9 +98,7 @@
     break;
   case llvm::Triple::Linux:
     module_spec = FileSpec("libc.so.6");
-    symbol_name = "__GI___assert_fail";
-    if (!ModuleHasDebugInfo(target, module_spec, symbol_name))
-      symbol_name = "__assert_fail";
+    symbol_name = "__assert_fail";
     break;
   default:
     Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_UNWIND));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D74252.243266.patch
Type: text/x-patch
Size: 995 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200207/55f957f8/attachment.bin>