From 92645447aec4ac2272b856f4f05022d430d39a4b Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Thu, 22 Aug 2024 09:50:44 -0300 Subject: [PATCH 1/3] Disable const propagation on externalized vars The value of the externalized vars are filled by the livepatch system, not the compiler. Hence if there are no writes into the variable the compiler won't generate the instructions to read it. As an example: ``` static int aa; int f(void) { return aa; } ``` generates: ``` xor eax, eax ret ``` Notice how there is no memory read. Adding the `__attribute__((used))` to it changes the asm code to: ``` movl aa(%rip), %eax ret ``` Fixing the issue. Signed-off-by: Giuliano Belinassi --- libcextract/SymbolExternalizer.cpp | 4 ++++ testsuite/ccp/ext-obj-5.c | 2 +- testsuite/includes/rewrite-1.c | 2 +- testsuite/includes/rewrite-2.c | 2 +- testsuite/includes/rewrite-4.c | 2 +- testsuite/lateext/lateext-2.c | 2 +- testsuite/lateext/lateext-3.c | 2 +- testsuite/lateext/lateext-4.c | 2 +- testsuite/lateext/lateext-5.c | 2 +- testsuite/lateext/lateext-6.c | 2 +- testsuite/lateext/lateext-7.c | 2 +- testsuite/lateext/location-1.c | 2 +- testsuite/small/rename-10.c | 2 +- testsuite/small/rename-12.c | 2 +- testsuite/small/rename-15.c | 2 +- testsuite/small/rename-2.c | 2 +- testsuite/small/rename-3.c | 2 +- testsuite/small/rename-4.c | 2 +- testsuite/small/rename-5.c | 2 +- testsuite/small/rename-6.c | 2 +- testsuite/small/rename-8.c | 2 +- testsuite/small/rename-9.c | 2 +- 22 files changed, 25 insertions(+), 21 deletions(-) diff --git a/libcextract/SymbolExternalizer.cpp b/libcextract/SymbolExternalizer.cpp index e91dc0e..02a495f 100644 --- a/libcextract/SymbolExternalizer.cpp +++ b/libcextract/SymbolExternalizer.cpp @@ -632,6 +632,10 @@ VarDecl *SymbolExternalizer::Create_Externalized_Var(DeclaratorDecl *decl, const sc ); + if (!Ibt) { + ret->addAttr(UsedAttr::Create(astctx)); + } + /* return node. */ return ret; } diff --git a/testsuite/ccp/ext-obj-5.c b/testsuite/ccp/ext-obj-5.c index eab6c49..390dbd2 100644 --- a/testsuite/ccp/ext-obj-5.c +++ b/testsuite/ccp/ext-obj-5.c @@ -12,5 +12,5 @@ void pu_f(void) ee_o; } -/* { dg-final { scan-tree-dump "static char \(\*klpe_ee_o\)\[4\];" } } */ +/* { dg-final { scan-tree-dump "static char \(\*klpe_ee_o\)\[4\] __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "char s\[sizeof\(\(\*klpe_ee_o\)\)\], t\[sizeof\(\(\*klpe_ee_o\)\)\]\[sizeof\(\(\*klpe_ee_o\)\)\];" } } */ diff --git a/testsuite/includes/rewrite-1.c b/testsuite/includes/rewrite-1.c index 2583ce1..22a596e 100644 --- a/testsuite/includes/rewrite-1.c +++ b/testsuite/includes/rewrite-1.c @@ -7,6 +7,6 @@ int g(void) return f(); } -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "\(\*klpe_f\)\(\)" } } */ /* { dg-final { scan-tree-dump-not "#include \"header-1.h\"" } } */ diff --git a/testsuite/includes/rewrite-2.c b/testsuite/includes/rewrite-2.c index 735c2eb..e808780 100644 --- a/testsuite/includes/rewrite-2.c +++ b/testsuite/includes/rewrite-2.c @@ -7,6 +7,6 @@ int g(void) return f(); } -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "\(\*klpe_f\)\(\)" } } */ /* { dg-final { scan-tree-dump-not "#include \"header-1.h\"" } } */ diff --git a/testsuite/includes/rewrite-4.c b/testsuite/includes/rewrite-4.c index 1d5a2ab..232f295 100644 --- a/testsuite/includes/rewrite-4.c +++ b/testsuite/includes/rewrite-4.c @@ -5,5 +5,5 @@ int g(void) return f(); } -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "\(\*klpe_f\)\(\)" } } */ diff --git a/testsuite/lateext/lateext-2.c b/testsuite/lateext/lateext-2.c index 042386e..e42ea03 100644 --- a/testsuite/lateext/lateext-2.c +++ b/testsuite/lateext/lateext-2.c @@ -10,5 +10,5 @@ int h(void) } /* { dg-final { scan-tree-dump "#define MACRO\n#include \"lateext-2.h\"\n#undef MACRO\n#include \"lateext-2.h\"" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_f\)\(\) \+ g\(\);" } } */ diff --git a/testsuite/lateext/lateext-3.c b/testsuite/lateext/lateext-3.c index 330b5e0..e9f531c 100644 --- a/testsuite/lateext/lateext-3.c +++ b/testsuite/lateext/lateext-3.c @@ -7,5 +7,5 @@ int f(void) return inline_func(); } -/* { dg-final { scan-tree-dump "static int \*klpe_var;" } } */ +/* { dg-final { scan-tree-dump "static int \*klpe_var __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump-not "#include \"lateext-3.h\"" } } */ diff --git a/testsuite/lateext/lateext-4.c b/testsuite/lateext/lateext-4.c index 779785c..90af762 100644 --- a/testsuite/lateext/lateext-4.c +++ b/testsuite/lateext/lateext-4.c @@ -10,5 +10,5 @@ MACRO f(void) } /* { dg-final { scan-tree-dump "#include \"lateext-4.h\"" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_g\)\(\);" } } */ diff --git a/testsuite/lateext/lateext-5.c b/testsuite/lateext/lateext-5.c index 6cf41da..fd65ce8 100644 --- a/testsuite/lateext/lateext-5.c +++ b/testsuite/lateext/lateext-5.c @@ -10,5 +10,5 @@ int f(void) } /* { dg-final { scan-tree-dump "#include \"lateext-4.h\"" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_g\)\(\);" } } */ diff --git a/testsuite/lateext/lateext-6.c b/testsuite/lateext/lateext-6.c index dfde21f..d318c42 100644 --- a/testsuite/lateext/lateext-6.c +++ b/testsuite/lateext/lateext-6.c @@ -11,5 +11,5 @@ int f(void) } /* { dg-final { scan-tree-dump-not "int g\(void\)" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_g\)\(\);" } } */ diff --git a/testsuite/lateext/lateext-7.c b/testsuite/lateext/lateext-7.c index 5217fcb..24d08bc 100644 --- a/testsuite/lateext/lateext-7.c +++ b/testsuite/lateext/lateext-7.c @@ -10,4 +10,4 @@ int f(void) } /* { dg-final { scan-tree-dump-not "DEFINE_STATIC_KEY_FALSE\(\*klpe_aaa\)" } } */ -/* { dg-final { scan-tree-dump "static struct AA \*klpe_aaa;" } } */ +/* { dg-final { scan-tree-dump "static struct AA \*klpe_aaa __attribute__\(\(used\)\);" } } */ diff --git a/testsuite/lateext/location-1.c b/testsuite/lateext/location-1.c index 8b28c5d..4e28bc0 100644 --- a/testsuite/lateext/location-1.c +++ b/testsuite/lateext/location-1.c @@ -7,5 +7,5 @@ int f(void) return global; } -/* { dg-final { scan-tree-dump "static int \*klpe_global;" } } */ +/* { dg-final { scan-tree-dump "static int \*klpe_global __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "clang-extract: from .*location-1.h:29:1" } } */ diff --git a/testsuite/small/rename-10.c b/testsuite/small/rename-10.c index fc03544..1a6210c 100644 --- a/testsuite/small/rename-10.c +++ b/testsuite/small/rename-10.c @@ -8,5 +8,5 @@ int g() return f(); } -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_f\)\(\);" } } */ diff --git a/testsuite/small/rename-12.c b/testsuite/small/rename-12.c index e14e45a..b502044 100644 --- a/testsuite/small/rename-12.c +++ b/testsuite/small/rename-12.c @@ -11,5 +11,5 @@ int f(void) } /* { dg-final { scan-tree-dump-not "static int g" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "int klpp_f\(void\)\n{\n *return \(\*klpe_g\)\(\)" } } */ diff --git a/testsuite/small/rename-15.c b/testsuite/small/rename-15.c index 5539c79..34233e4 100644 --- a/testsuite/small/rename-15.c +++ b/testsuite/small/rename-15.c @@ -18,4 +18,4 @@ int f(void) } /* { dg-final { scan-tree-dump "return \(\*klpe_g\)\(3\);" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(int\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(int\) __attribute__\(\(used\)\);" } } */ diff --git a/testsuite/small/rename-2.c b/testsuite/small/rename-2.c index ead0987..b45094f 100644 --- a/testsuite/small/rename-2.c +++ b/testsuite/small/rename-2.c @@ -11,5 +11,5 @@ int f(void) } /* { dg-final { scan-tree-dump-not "static int g" } } */ -/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_g\)\(\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "int f\(void\)\n{\n *return \(\*klpe_g\)\(\)" } } */ diff --git a/testsuite/small/rename-3.c b/testsuite/small/rename-3.c index 30e19a6..7098a16 100644 --- a/testsuite/small/rename-3.c +++ b/testsuite/small/rename-3.c @@ -7,5 +7,5 @@ int g(void) return f(); } -/* { dg-final { scan-tree-dump "static int \(\*\*klpe_f\)\(void\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*\*klpe_f\)\(void\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "int g\(void\)\n{\n *return \(\*klpe_f\)\(\)" } } */ diff --git a/testsuite/small/rename-4.c b/testsuite/small/rename-4.c index 2084ee6..260a5cb 100644 --- a/testsuite/small/rename-4.c +++ b/testsuite/small/rename-4.c @@ -12,5 +12,5 @@ int g() return f(); } -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_f\)\(\);" } } */ diff --git a/testsuite/small/rename-5.c b/testsuite/small/rename-5.c index f14b6a8..eb6c51f 100644 --- a/testsuite/small/rename-5.c +++ b/testsuite/small/rename-5.c @@ -1,5 +1,5 @@ /* { dg-options "-DCE_EXTRACT_FUNCTIONS=g -DCE_EXPORT_SYMBOLS=f" }*/ #include "rename-5.h" -/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\);" } } */ +/* { dg-final { scan-tree-dump "static int \(\*klpe_f\)\(\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_f\)\(\);" } } */ diff --git a/testsuite/small/rename-6.c b/testsuite/small/rename-6.c index 80f82d4..e0be681 100644 --- a/testsuite/small/rename-6.c +++ b/testsuite/small/rename-6.c @@ -11,6 +11,6 @@ int g() return a; } -/* { dg-final { scan-tree-dump "static int \*klpe_a;" } } */ +/* { dg-final { scan-tree-dump "static int \*klpe_a __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "\(\*klpe_a\) = x;" } } */ /* { dg-final { scan-tree-dump "return \(\*klpe_a\);" } } */ diff --git a/testsuite/small/rename-8.c b/testsuite/small/rename-8.c index 1d60f86..71ead10 100644 --- a/testsuite/small/rename-8.c +++ b/testsuite/small/rename-8.c @@ -10,6 +10,6 @@ int f() return A + B ; } -/* { dg-final { scan-tree-dump "static int \*klpe_bbb;" } } */ +/* { dg-final { scan-tree-dump "static int \*klpe_bbb __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "#define A \(\*klpe_bbb\)" } } */ /* { dg-final { scan-tree-dump "#define B \(\*klpe_bbb\)" } } */ diff --git a/testsuite/small/rename-9.c b/testsuite/small/rename-9.c index 36034e9..b2c75f2 100644 --- a/testsuite/small/rename-9.c +++ b/testsuite/small/rename-9.c @@ -10,5 +10,5 @@ int f() return OPENSSL_strdup("aaa"); } -/* { dg-final { scan-tree-dump "static char \(\*klpe_CRYPTO_strdup\)\(const char \*, const char \*, int\);" } } */ +/* { dg-final { scan-tree-dump "static char \(\*klpe_CRYPTO_strdup\)\(const char \*, const char \*, int\) __attribute__\(\(used\)\);" } } */ /* { dg-final { scan-tree-dump "\(\*klpe_CRYPTO_strdup\)\(str," } } */ From 88f7d523007ce8cb78da2c9234e8fa48f5cdcbfe Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Thu, 22 Aug 2024 11:29:10 -0300 Subject: [PATCH 2/3] Fix symbol visibility In the case the symbol is only available on SYMTAB we can't weakly externalize it, as the linker will resolve it as not found. Signed-off-by: Giuliano Belinassi --- libcextract/ElfCXX.cpp | 24 +++++++++++++ libcextract/ElfCXX.hh | 12 +++++++ libcextract/InlineAnalysis.cpp | 59 ++++++++++++++++++++----------- libcextract/InlineAnalysis.hh | 4 ++- testsuite/inline/inlined-into-1.c | 15 +++++--- 5 files changed, 88 insertions(+), 26 deletions(-) diff --git a/libcextract/ElfCXX.cpp b/libcextract/ElfCXX.cpp index 7e50d3d..a5a1223 100644 --- a/libcextract/ElfCXX.cpp +++ b/libcextract/ElfCXX.cpp @@ -365,6 +365,30 @@ ElfSymbolCache::ElfSymbolCache(ElfObject &eo) } } +/** Get symbol if available in either symtab. Returns in which symtab this + symbol was found. */ +std::pair +ElfSymbolCache::Get_Symbol_Info(const std::string &sym) +{ + unsigned char ret = 0; + + /* Try the dynsym first, which means this symbol is most likely public + visible. */ + ret = Get_Symbol_Info_Dynsym(sym); + if (ret != 0) { + return std::make_pair(ret, ElfSymtabType::DYNSYM); + } + + /* Symbol not available on dynsym. Try symtab. */ + ret = Get_Symbol_Info_Symtab(sym); + if (ret != 0) { + return std::make_pair(ret, ElfSymtabType::SYMTAB); + } + + /* If symbol is not here there is nothing I can do. */ + return std::make_pair(0, ElfSymtabType::TAB_NONE); +} + std::vector ElfSymbolCache::Get_All_Symbols(void) { std::vector vec; diff --git a/libcextract/ElfCXX.hh b/libcextract/ElfCXX.hh index dbbc89b..35dbbdc 100644 --- a/libcextract/ElfCXX.hh +++ b/libcextract/ElfCXX.hh @@ -38,6 +38,14 @@ class ElfObject; class ElfSection; class ElfSymbol; + +/** The symbol table in which this symbol was found. */ +enum ElfSymtabType { + TAB_NONE = 0, + SYMTAB = SHT_SYMTAB, + DYNSYM = SHT_DYNSYM, +}; + /** @brief ELF symbol. * * An ELF executable may contain multiple symbols (variables, functions, ...). @@ -358,6 +366,10 @@ class ElfSymbolCache return 0; } + /** Get symbol if available in either symtab. Returns in which symtab this + symbo was found. */ + std::pair Get_Symbol_Info(const std::string &sym); + std::string Get_Symbol_Module(const std::string &) { return Mod; diff --git a/libcextract/InlineAnalysis.cpp b/libcextract/InlineAnalysis.cpp index c8238e3..06a2c7f 100644 --- a/libcextract/InlineAnalysis.cpp +++ b/libcextract/InlineAnalysis.cpp @@ -162,7 +162,9 @@ void InlineAnalysis::Print_Node_Colors(const std::set &set, FILE for (IpaCloneNode *node : set) { const char *demangled = InlineAnalysis::Demangle_Symbol(node->Name.c_str()); - unsigned char syminfo = Get_Symbol_Info(node->Name); + std::pair infos = Get_Symbol_Info(node->Name); + unsigned char syminfo = infos.first; + ElfSymtabType symtab = infos.second; if (syminfo == 0) { fprintf(fp, "\n\"%s\" [style=dotted]", demangled); } else { @@ -170,8 +172,10 @@ void InlineAnalysis::Print_Node_Colors(const std::set &set, FILE unsigned char bind = ElfSymbol::Bind_Of(syminfo); if (bind == STB_LOCAL) { fprintf(fp, "\n\"%s\" [color=red]", demangled); - } else if (bind == STB_GLOBAL) { + } else if (bind == STB_GLOBAL && symtab == SHT_DYNSYM) { fprintf(fp, "\n\"%s\" [color=black]", demangled); + } else if (bind == STB_GLOBAL && symtab == SHT_SYMTAB) { + fprintf(fp, "\n\"%s\" [color=red]", demangled); } else if (bind == STB_WEAK) { fprintf(fp, "\n\"%s\" [color=green]", demangled); } else { @@ -247,29 +251,21 @@ void InlineAnalysis::Dump(void) } } -unsigned char InlineAnalysis::Get_Symbol_Info(const std::string &sym) +std::pair +InlineAnalysis::Get_Symbol_Info(const std::string &sym) { /* Try the dynsym first, which means this symbol is most likely publically visible. */ - unsigned char ret = 0; if (ElfCache == nullptr) { /* If we don't have the ElfCache there is nothing we can do. */ - return ret; - } - - ret = ElfCache->Get_Symbol_Info_Dynsym(sym); - if (ret != 0) { - return ret; + return std::make_pair(0, ElfSymtabType::TAB_NONE); } - /* Symbol not available on dynsym. Try symtab. */ - ret = ElfCache->Get_Symbol_Info_Symtab(sym); - - /* If symbol is not here there is nothing I can do. */ - return ret; + /* Forward this to ElfCXX. */ + return ElfCache->Get_Symbol_Info(sym); } -static const char *Bind(unsigned link) +static const char *Bind(unsigned link, ElfSymtabType symtab) { switch (link) { case STB_LOCAL: @@ -277,7 +273,13 @@ static const char *Bind(unsigned link) break; case STB_GLOBAL: - return "Public symbol"; + if (symtab == SHT_SYMTAB) { + return "Private symbol"; + } else if (symtab == SHT_DYNSYM) { + return "Public symbol"; + } else { + assert(0 && "Unreachable"); + } break; case STB_WEAK: @@ -311,7 +313,9 @@ ExternalizationType InlineAnalysis::Needs_Externalization(const std::string &sym : ExternalizationType::NONE; } - unsigned char info = Get_Symbol_Info(sym); + auto infos = Get_Symbol_Info(sym); + unsigned char info = infos.first; + ElfSymtabType symtab = infos.second; if (info > 0) { unsigned bind = ElfSymbol::Bind_Of(info); switch (bind) { @@ -325,7 +329,17 @@ ExternalizationType InlineAnalysis::Needs_Externalization(const std::string &sym if (Kernel) { return ExternalizationType::STRONG; } - return ExternalizationType::WEAK; + + /* We need to be careful with the symbol table this came from. If this + is from SYMTAB we can't weakly externalize it. */ + if (symtab == SHT_SYMTAB) { + return ExternalizationType::STRONG; + } else if (symtab == SHT_DYNSYM) { + return ExternalizationType::WEAK; + } else { + assert(0 && "Unreachable."); + } + break; case STB_LOCAL: return ExternalizationType::STRONG; @@ -455,12 +469,15 @@ void InlineAnalysis::Print_Symbol_Set(const std::set &symbol_set, /* In case we have debuginfo, also print if symbols was completely inlined. */ if (have_debuginfo) { - unsigned char syminfo = Get_Symbol_Info(s); + auto syminfos = Get_Symbol_Info(s); + unsigned char syminfo = syminfos.first; + ElfSymtabType symtab = syminfos.second; + unsigned type = ElfSymbol::Type_Of(syminfo); unsigned bind = ElfSymbol::Bind_Of(syminfo); const char *type_str = ElfSymbol::Type_As_String(type); - const char *bind_str = (syminfo > 0) ? Bind(bind) : "Inlined"; + const char *bind_str = (syminfo > 0) ? Bind(bind, symtab) : "Inlined"; if (csv) { fprintf(out,"%s;%s\n", type_str, bind_str); diff --git a/libcextract/InlineAnalysis.hh b/libcextract/InlineAnalysis.hh index 97eb9eb..c3aeb71 100644 --- a/libcextract/InlineAnalysis.hh +++ b/libcextract/InlineAnalysis.hh @@ -74,7 +74,9 @@ class InlineAnalysis const char *output_path); /** Get the ELF info of a symbol. */ - unsigned char Get_Symbol_Info(const std::string &sym); + + std::pair + Get_Symbol_Info(const std::string &sym); ExternalizationType Needs_Externalization(const std::string &sym); diff --git a/testsuite/inline/inlined-into-1.c b/testsuite/inline/inlined-into-1.c index bbdcd51..333ff91 100644 --- a/testsuite/inline/inlined-into-1.c +++ b/testsuite/inline/inlined-into-1.c @@ -1,4 +1,4 @@ -/* { dg-compile "-fdump-ipa-clones -O3 -g3"} */ +/* { dg-compile "-fdump-ipa-clones -O3 -g3 -shared"} */ /* { dg-options "-where-is-inlined g"} */ static inline int g(void) @@ -6,17 +6,24 @@ static inline int g(void) return 42; } -int f() +static __attribute__((noinline)) h(void) { return g(); } +int f() +{ + return h() + g(); +} + int main(void) { - return f(); + return f() + g(); } /* { dg-final { scan-tree-dump "f" } } */ /* { dg-final { scan-tree-dump "g" } } */ -/* { dg-final { scan-tree-dump "FUNC\tPublic" } } */ +/* { dg-final { scan-tree-dump "f *.*FUNC\tPublic symbol\n" } } */ +/* { dg-final { scan-tree-dump "h *.*FUNC\tPrivate symbol\n" } } */ +/* { dg-final { scan-tree-dump "main *.*FUNC\tPublic symbol\n" } } */ From 74de718f368ab81bf531f6a4b9446259a2324799 Mon Sep 17 00:00:00 2001 From: Giuliano Belinassi Date: Thu, 22 Aug 2024 18:03:51 -0300 Subject: [PATCH 3/3] Add option to pass two debuginfos SUSE packages contain the stripped .so file with .dynsym and the debuginfo (.debug) provided in -debuginfo packages. We need both of them in Userspace Livepatching since 15.5 for proper analysis. This commit patches the ElfCache and InlineAnalysis so it can use both of them. For kernel we expect it to only use one ELF file for now (the first one). Signed-off-by: Giuliano Belinassi --- libcextract/ArgvParser.cpp | 9 +++++++-- libcextract/ArgvParser.hh | 7 ++++--- libcextract/ElfCXX.cpp | 15 +++++++++++++-- libcextract/ElfCXX.hh | 29 +++++++++++++++++++++++++++-- libcextract/InlineAnalysis.cpp | 23 ++++++++++++++--------- libcextract/InlineAnalysis.hh | 30 ++++++++++++++++++++++++++---- libcextract/Passes.hh | 6 +++--- 7 files changed, 94 insertions(+), 25 deletions(-) diff --git a/libcextract/ArgvParser.cpp b/libcextract/ArgvParser.cpp index c428aca..1686a6b 100644 --- a/libcextract/ArgvParser.cpp +++ b/libcextract/ArgvParser.cpp @@ -74,7 +74,7 @@ ArgvParser::ArgvParser(int argc, char **argv) Ibt(false), AllowLateExternalization(false), PatchObject(""), - DebuginfoPath(nullptr), + Debuginfos(), IpaclonesPath(nullptr), SymversPath(nullptr), DescOutputPath(nullptr), @@ -89,6 +89,11 @@ ArgvParser::ArgvParser(int argc, char **argv) Insert_Required_Parameters(); + const char *DebuginfoPath = nullptr; + if (Debuginfos.size() > 0) { + DebuginfoPath = Debuginfos[0].c_str(); + } + /* For kernel, check if the object patch is not the same as DebugInfo. If they * are not the same, it means that the module from PatchObject is builtin, so * assign vmlinux to PatchObject. */ @@ -252,7 +257,7 @@ bool ArgvParser::Handle_Clang_Extract_Arg(const char *str) return true; } if (prefix("-DCE_DEBUGINFO_PATH=", str)) { - DebuginfoPath = Extract_Single_Arg_C(str); + Debuginfos = Extract_Args(str); return true; } diff --git a/libcextract/ArgvParser.hh b/libcextract/ArgvParser.hh index 77546b5..c0a85eb 100644 --- a/libcextract/ArgvParser.hh +++ b/libcextract/ArgvParser.hh @@ -94,9 +94,9 @@ class ArgvParser return PatchObject; } - inline const char *Get_Debuginfo_Path(void) + inline std::vector &Get_Debuginfo_Path(void) { - return DebuginfoPath; + return Debuginfos; } inline const char *Get_Ipaclones_Path(void) @@ -163,7 +163,8 @@ class ArgvParser bool AllowLateExternalization; std::string PatchObject; - const char *DebuginfoPath; + std::vector Debuginfos; + const char *IpaclonesPath; const char *SymversPath; diff --git a/libcextract/ElfCXX.cpp b/libcextract/ElfCXX.cpp index a5a1223..d3c7f4c 100644 --- a/libcextract/ElfCXX.cpp +++ b/libcextract/ElfCXX.cpp @@ -335,19 +335,30 @@ void ElfSymbolCache::Insert_Symbols_Into_Hash(SymbolTableHash &map, ElfSection & } ElfSymbolCache::ElfSymbolCache(ElfObject &eo) - : EO(eo) + : ElfSymbolCache() +{ + Analyze_ELF(eo); +} + +void ElfSymbolCache::Analyze_ELF(ElfObject &eo) { /* Look for dynsym and symtab sections. */ - for (auto it = EO.section_begin(); it != EO.section_end(); ++it) + for (auto it = eo.section_begin(); it != eo.section_end(); ++it) { ElfSection §ion = *it; switch (section.Get_Section_Type()) { case SHT_DYNSYM: Insert_Symbols_Into_Hash(DynsymMap, section); + + assert(ObjectPath == "" && "Multiple libraries passed as debuginfo?"); + ObjectPath = eo.Get_Path(); break; case SHT_SYMTAB: Insert_Symbols_Into_Hash(SymtabMap, section); + + /* We can also have symtab in the .so library. */ + DebuginfoPath = eo.Get_Path(); break; case SHT_PROGBITS: diff --git a/libcextract/ElfCXX.hh b/libcextract/ElfCXX.hh index 35dbbdc..422a503 100644 --- a/libcextract/ElfCXX.hh +++ b/libcextract/ElfCXX.hh @@ -346,6 +346,18 @@ class ElfSymbolCache /** Build cache from ElfObject). */ ElfSymbolCache(ElfObject &eo); + /** Build empty cache to be filled later. */ + ElfSymbolCache(void) + : DynsymMap(), + SymtabMap(), + Mod(""), + DebuginfoPath(""), + ObjectPath("") + {}; + + /** Analyze ELF object. */ + void Analyze_ELF(ElfObject &eo); + /* Get symbol if available in the Dynsym table. Or 0 if not available. */ inline unsigned char Get_Symbol_Info_Dynsym(const std::string &sym) { @@ -377,6 +389,16 @@ class ElfSymbolCache std::vector Get_All_Symbols(void); + inline const std::string &Get_Debuginfo_Path(void) const + { + return DebuginfoPath; + } + + inline const std::string &Get_Object_Path(void) const + { + return ObjectPath; + } + /** Dump for debugging reasons. */ void Dump_Cache(void); @@ -394,6 +416,9 @@ class ElfSymbolCache /** Kernel module name, if .modinfo section is present. */ std::string Mod; - /** Reference to the ElfObject used to build this object. */ - ElfObject &EO; + /** Path to the debuginfo object (contains .symtab section). */ + std::string DebuginfoPath; + + /** Path to the .so file object (contains .dynsym section). */ + std::string ObjectPath; }; diff --git a/libcextract/InlineAnalysis.cpp b/libcextract/InlineAnalysis.cpp index 06a2c7f..6067734 100644 --- a/libcextract/InlineAnalysis.cpp +++ b/libcextract/InlineAnalysis.cpp @@ -22,10 +22,10 @@ #include #include -InlineAnalysis::InlineAnalysis(const char *elf_path, const char *ipaclones_path, - const char *symvers_path, bool is_kernel) - : ElfObj(nullptr), - ElfCache(nullptr), +InlineAnalysis::InlineAnalysis(const std::vector &elfs_path, + const char *ipaclones_path, + const char *symvers_path, bool is_kernel) + : ElfCache(nullptr), Ipa(nullptr), Symv(nullptr), Kernel(is_kernel) @@ -33,9 +33,16 @@ InlineAnalysis::InlineAnalysis(const char *elf_path, const char *ipaclones_path, try { /* Debuginfo information is not needed for inline analysis. But is desired for better precision. That is why whe declare those objects dynamically. */ - if (elf_path) { - ElfObj = new ElfObject(elf_path); - ElfCache = new ElfSymbolCache(*ElfObj); + if (elfs_path.size() > 0) { + /* Only create an ElfSymbolCache if we received elfs from command line. */ + ElfCache = new ElfSymbolCache(); + } + + /* Initialize the SymbolCache. */ + for (auto it = elfs_path.begin(); it != elfs_path.end(); ++it) { + const std::string &path = *it; + ElfObject elf(path); + ElfCache->Analyze_ELF(elf); } if (ipaclones_path) { @@ -62,8 +69,6 @@ InlineAnalysis::~InlineAnalysis(void) { if (ElfCache) delete ElfCache; - if (ElfObj) - delete ElfObj; if (Ipa) delete Ipa; if (Symv) diff --git a/libcextract/InlineAnalysis.hh b/libcextract/InlineAnalysis.hh index c3aeb71..2076040 100644 --- a/libcextract/InlineAnalysis.hh +++ b/libcextract/InlineAnalysis.hh @@ -48,7 +48,15 @@ class InlineAnalysis available, and ipaclone_path can be a directory full of many ipa-clones generated through LTO or not. Symvers can be NULL is we are creating a userspace livepatch */ - InlineAnalysis(const char *elf_path, const char *ipaclone_path, const char *symvers_path, bool is_kernel); + InlineAnalysis(const std::vector &elfs_path, + const char *ipaclone_path, const char *symvers_path, + bool is_kernel); + + InlineAnalysis(const char *elf_path, const char *ipaclone_path, + const char *symvers_path, bool is_kernel) + : InlineAnalysis(std::vector({elf_path}), + ipaclone_path, symvers_path, is_kernel) + {} ~InlineAnalysis(void); @@ -93,12 +101,27 @@ class InlineAnalysis /** True if this class was built with debuginfo enabled. */ inline bool Have_Debuginfo(void) { - return ElfObj && ElfCache; + return ElfCache; } inline const std::string &Get_Debuginfo_Path(void) { - return ElfObj->Get_Path(); + if (ElfCache) { + return ElfCache->Get_Debuginfo_Path(); + } + + static const std::string empty; + return empty; + } + + inline const std::string &Get_Object_Path(void) + { + if (ElfCache) { + return ElfCache->Get_Object_Path(); + } + + static const std::string empty; + return empty; } /** Check if we have Ipa-clones information. */ @@ -143,7 +166,6 @@ class InlineAnalysis /** Put color information in the graphviz .DOT file. */ void Print_Node_Colors(const std::set &set, FILE *fp); - ElfObject *ElfObj; ElfSymbolCache *ElfCache; IpaClones *Ipa; Symvers *Symv; diff --git a/libcextract/Passes.hh b/libcextract/Passes.hh index 4f2a295..0e52fbc 100644 --- a/libcextract/Passes.hh +++ b/libcextract/Passes.hh @@ -58,7 +58,7 @@ class PassManager { PatchObject(args.Get_PatchObject()), HeadersToExpand(args.Get_Headers_To_Expand()), ClangArgs(args.Get_Args_To_Clang()), - DebuginfoPath(args.Get_Debuginfo_Path()), + Debuginfos(args.Get_Debuginfo_Path()), IpaclonesPath(args.Get_Ipaclones_Path()), SymversPath(args.Get_Symvers_Path()), DscOutputPath(args.Get_Dsc_Output_Path()), @@ -67,7 +67,7 @@ class PassManager { args.Get_Include_Expansion_Policy(), Kernel)), NamesLog(), PassNum(0), - IA(DebuginfoPath, IpaclonesPath, SymversPath, args.Is_Kernel()) + IA(Debuginfos, IpaclonesPath, SymversPath, args.Is_Kernel()) { } @@ -121,7 +121,7 @@ class PassManager { std::vector &ClangArgs; /* Path to Debuginfo, if exists. */ - const char *DebuginfoPath; + std::vector &Debuginfos; /* Path to Ipaclones, if exists. */ const char *IpaclonesPath;