From 6aed34a5c70b1f23fcfa20cbfd4827bbab11f001 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Fri, 21 Sep 2018 18:03:20 +0100 Subject: [PATCH] Annobin: Generate notes and groups for .text.hot and .text.unlikely sections. When -ffunction-sections is active, put notes for startup sections into .text.startup.foo rather than .text.foo. Similarly put exit section notes into .text.exit.foo. Annocheck: Update version to 8.38 Change maybe result for GNU Property note being missing into a PASS if it is not needed and a FAIL if it is needed. Update tests to take into account these changes. --- annocheck/annocheck.c | 2 +- annocheck/hardened.c | 21 +- plugin/annobin.cc | 387 +++++++++++++++++++---------------- tests/function-sections-test | 14 -- tests/missing-notes-test | 2 +- tests/unused_code.c | 19 +- 6 files changed, 249 insertions(+), 196 deletions(-) diff --git a/annocheck/annocheck.c b/annocheck/annocheck.c index 6a23996..8b63323 100644 --- a/annocheck/annocheck.c +++ b/annocheck/annocheck.c @@ -29,7 +29,7 @@ ulong verbosity = 0; uint major_version = 8; -uint minor_version = 37; +uint minor_version = 38; static ulong num_files = 0; static const char * files[MAX_NUM_FILES]; diff --git a/annocheck/hardened.c b/annocheck/hardened.c index 8684931..2fef76b 100644 --- a/annocheck/hardened.c +++ b/annocheck/hardened.c @@ -1159,6 +1159,8 @@ interesting_seg (annocheck_data * data, break; case PT_NOTE: + if (skip_check (TEST_PROPERTY_NOTE, NULL)) + break; /* We want to examine the note segments on x86_64 binaries. */ return (e_machine == EM_X86_64); @@ -1184,7 +1186,7 @@ check_seg (annocheck_data * data, size_t offset = 0; offset = gelf_getnote (seg->data, offset, & note, & name_off, & data_off); - + if (seg->phdr->p_align != 8) { if (seg->phdr->p_align != 4) @@ -1202,11 +1204,16 @@ check_seg (annocheck_data * data, } } - if (note.n_type == NT_GNU_PROPERTY_TYPE_0 && offset != 0) + if (note.n_type == NT_GNU_PROPERTY_TYPE_0) { - einfo (VERBOSE, "%s: More than one GNU Property note in note segment", - data->filename); - tests[TEST_PROPERTY_NOTE].num_fail ++; + if (offset != 0) + { + einfo (VERBOSE, "%s: More than one GNU Property note in note segment", + data->filename); + tests[TEST_PROPERTY_NOTE].num_fail ++; + } + else + tests[TEST_PROPERTY_NOTE].num_pass ++; } return true; @@ -1520,8 +1527,10 @@ show_PROPERTY_NOTE (annocheck_data * data, test * results) maybe (data, "Corrupt GNU Property note"); else if (results->num_pass > 0) pass (data, "Good GNU Property note"); + else if (tests[TEST_CF_PROTECTION].enabled && tests[TEST_CF_PROTECTION].num_pass > 0) + fail (data, "GNU Property note is missing, but -fcf-protection is enabled"); else - fail (data, "GNU Property note is missing"); + pass (data, "GNU Property note not needed"); } static void diff --git a/plugin/annobin.cc b/plugin/annobin.cc index 92348a5..8cf0efb 100644 --- a/plugin/annobin.cc +++ b/plugin/annobin.cc @@ -862,93 +862,16 @@ annobin_get_node (const_tree decl) #endif } -/* Create any notes specific to the current function. */ - static void -annobin_create_function_notes (void * gcc_data, void * user_data) +annobin_create_function_symbols_and_notes (const char * func_section, + const char * func_name, + const char * asm_name) { - const char * sec_name = NULL; - const char * func_section; - const char * func_name; - const char * asm_name; - const char * start_sym; - const char * end_sym; - bool force; - unsigned int count; - - if (saved_end_sym != NULL) - annobin_inform (0, "XXX ICE: end sym %s not NULL\n", saved_end_sym); - - if (! annobin_enable_static_notes || asm_out_file == NULL) - return; - - func_name = current_function_name (); - asm_name = function_asm_name (); - - if (func_name == NULL) - { - func_name = asm_name; - - if (func_name == NULL) - { - /* Can this happen ? */ - annobin_inform (0, "ICE: function name not available"); - return; - } - } - - if (asm_name == NULL) - asm_name = func_name; - - func_section = annobin_get_section_name (current_function_decl); - if (func_section != NULL) - /* This is just so that we can free func_section at the end of this code. */ - func_section = concat (func_section, NULL); - - else if (flag_function_sections) - func_section = concat (".text.", asm_name, NULL); - - else if (flag_reorder_functions /* && targetm_common.have_named_sections*/) - { - struct cgraph_node * node = annobin_get_node (current_function_decl); - - if (node) - { - bool startup = node->only_called_at_startup; - bool exit = node->only_called_at_exit; - - /* Attempt to determine the section into which the code will be placed. - We could call targetm.asm_out_function_section but that ends up calling - get_section() which will *create* a section if none exists. This causes - problems because later on gcc will attempt to create the section again - but this time it might be using different flags. - - So instead we duplicate the code in gcc/varasm.c:default_function_section() - except that we do not actually call get_named_text_section(). */ - - if (node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED) - func_section = concat (".text.unlikely", NULL); - else if (startup) - { - if (!in_lto_p && ! flag_profile_values) - func_section = concat (".text.startup", NULL); - } - else if (exit) - { - func_section = concat (".text.exit", NULL); - } - else if (node->frequency == NODE_FREQUENCY_HOT) - { - if (!in_lto_p && ! flag_profile_values) - func_section = concat (".text.hot", NULL); - } - else if (DECL_COMDAT_GROUP (current_function_decl)) - { - targetm.asm_out.unique_section (current_function_decl, 0); - func_section = concat (annobin_get_section_name (current_function_decl), NULL); - } - } - } + const char * sec_name = NULL; + const char * start_sym; + const char * end_sym; + unsigned int count; + bool force; /* If the function is going to be in its own section, then we do not know where it will end up in memory. In particular we cannot rely upon it @@ -962,11 +885,11 @@ annobin_create_function_notes (void * gcc_data, void * user_data) { /* Include a group name in our section name. */ if (strstr (func_section, ".gnu.linkonce.")) - sec_name = concat (GNU_BUILD_ATTRS_SECTION_NAME, ".", func_section, ", \"G\", %note, ", - func_section, ".group, comdat", NULL); + sec_name = concat (GNU_BUILD_ATTRS_SECTION_NAME, ".", func_section, + ", \"G\", %note, ", func_section, ".group, comdat", NULL); else - sec_name = concat (GNU_BUILD_ATTRS_SECTION_NAME, ".", func_section, ", \"G\", %note, ", - func_section, ".group", NULL); + sec_name = concat (GNU_BUILD_ATTRS_SECTION_NAME, ".", func_section, + ", \"G\", %note, ", func_section, ".group", NULL); } else sec_name = concat (GNU_BUILD_ATTRS_SECTION_NAME, NULL); @@ -974,13 +897,13 @@ annobin_create_function_notes (void * gcc_data, void * user_data) /* We use our own function start and end symbols so that they will not interfere with the program proper. In particular if we use the function name symbol ourselves then we can cause problems - when the linker attempts to relocs against it and finds that it - has both PC relative and abolsute relocs. - + when the linker attempts to resolve relocs against it and finds + that it has both PC relative and abolsute relocs. + We try our best to ensure that the new symbols will not clash with any other symbols in the program. */ start_sym = concat (ANNOBIN_SYMBOL_PREFIX, asm_name, ".start", NULL); - end_sym = concat (ANNOBIN_SYMBOL_PREFIX, asm_name, ".end", NULL); + end_sym = concat (ANNOBIN_SYMBOL_PREFIX, asm_name, ".end", NULL); count = annobin_note_count; annobin_emit_function_notes (func_name, start_sym, end_sym, sec_name, force); @@ -1012,25 +935,6 @@ annobin_create_function_notes (void * gcc_data, void * user_data) fprintf (asm_out_file, "\t.hidden %s\n", start_sym); fprintf (asm_out_file, "%s:\n", start_sym); fprintf (asm_out_file, "\t.popsection\n"); -#if 0 - /* If the function is in a linkonce section then it is possible that - it will be removed by the linker. In that case we could be left - with a dangling reference from the annobin notes to the now - deleted annobin symbols in the function section. So we provide - weak definitions of the symbols here. We also make them - hidden in order to indicate that they are not needed elsewhere. - - FIXME - Do we need to worry about COMDAT code sections ? */ - if (strstr (func_section, ".gnu.linkonce.")) - { - fprintf (asm_out_file, "\t.pushsection %s\n", sec_name); - fprintf (asm_out_file, "\t.weak %s\n", start_sym); - fprintf (asm_out_file, "\t.hidden %s\n", start_sym); - fprintf (asm_out_file, "\t.weak %s\n", end_sym); - fprintf (asm_out_file, "\t.hidden %s\n", end_sym); - fprintf (asm_out_file, "\t.popsection\n"); - } -#endif } saved_end_sym = end_sym; @@ -1041,10 +945,115 @@ annobin_create_function_notes (void * gcc_data, void * user_data) } free ((void *) start_sym); - free ((void *) func_section); free ((void *) sec_name); } +/* Create any notes specific to the current function. */ + +static void +annobin_create_function_notes (void * gcc_data, void * user_data) +{ + const char * func_section; + const char * func_name; + const char * asm_name; + + if (saved_end_sym != NULL) + annobin_inform (0, "XXX ICE: end sym %s not NULL\n", saved_end_sym); + + if (! annobin_enable_static_notes || asm_out_file == NULL) + return; + + func_name = current_function_name (); + asm_name = function_asm_name (); + + if (func_name == NULL) + { + func_name = asm_name; + + if (func_name == NULL) + { + /* Can this happen ? */ + annobin_inform (0, "ICE: function name not available"); + return; + } + } + + if (asm_name == NULL) + asm_name = func_name; + + struct cgraph_node * node = annobin_get_node (current_function_decl); + bool startup, exit, unlikely, likely; + + if (node) + { + startup = node->only_called_at_startup; + exit = node->only_called_at_exit; + unlikely = node->frequency == NODE_FREQUENCY_UNLIKELY_EXECUTED; + node->frequency == NODE_FREQUENCY_HOT; + } + else + startup = exit = unlikely = likely = false; + + func_section = annobin_get_section_name (current_function_decl); + if (func_section != NULL) + /* This is just so that we can free func_section at the end of this code. */ + func_section = concat (func_section, NULL); + + else if (flag_function_sections) + { + /* Special case: at -O2 or higher startup and exit functions get the prefix added. */ + if (startup && flag_reorder_functions) + func_section = concat (".text.startup.", asm_name, NULL); + else if (exit && flag_reorder_functions) + func_section = concat (".text.exit.", asm_name, NULL); + + else + func_section = concat (".text.", asm_name, NULL); + } + + else if (flag_reorder_functions /* && targetm_common.have_named_sections */) + { + /* Attempt to determine the section into which the code will be placed. + We could call targetm.asm_out_function_section but that ends up calling + get_section() which will *create* a section if none exists. This causes + problems because later on gcc will attempt to create the section again + but this time it might be using different flags. + + So instead we duplicate the code in gcc/varasm.c:default_function_section() + except that we do not actually call get_named_text_section(). */ + + if (unlikely) + { + /* FIXME: Never actually seen this case occur... */ + func_section = concat (".text.unlikely", NULL); + } + else if (startup) + { + if (!in_lto_p && ! flag_profile_values) + func_section = concat (".text.startup.", asm_name, NULL); + } + else if (exit) + { + func_section = concat (".text.exit.", asm_name, NULL); + } + else if (likely) + { + /* FIXME: Never seen this one, either. */ + if (!in_lto_p && ! flag_profile_values) + func_section = concat (".text.hot.", asm_name, NULL); + } + else if (DECL_COMDAT_GROUP (current_function_decl)) + { + targetm.asm_out.unique_section (current_function_decl, 0); + func_section = concat (annobin_get_section_name (current_function_decl), NULL); + } + } + + annobin_create_function_symbols_and_notes (func_section, func_name, asm_name); + + free ((void *) func_section); +} + static void annobin_create_function_end_symbol (void * gcc_data, void * user_data) @@ -1052,6 +1061,8 @@ annobin_create_function_end_symbol (void * gcc_data, void * user_data) if (! annobin_enable_static_notes || asm_out_file == NULL) return; + struct cgraph_node * node = annobin_get_node (current_function_decl); + if (saved_end_sym) { const char * sn = annobin_get_section_name (current_function_decl); @@ -1069,6 +1080,73 @@ annobin_create_function_end_symbol (void * gcc_data, void * user_data) } } +static void +annobin_emit_start_sym_and_version_note (const char * suffix, + const char producer_char) +{ + if (* suffix) + /* We put suffixed text sections into a group so that the linker + can delete the notes if the code is discarded. */ + fprintf (asm_out_file, "\t.pushsection .text%s, \"axG\", %%progbits, text%s.group\n", suffix, suffix); + else + fprintf (asm_out_file, "\t.pushsection .text\n"); + + fprintf (asm_out_file, "\t%s %s%s\n", global_file_name_symbols ? ".global" : ".hidden", + annobin_current_filename, suffix); + + /* Note - we used to set the type of the symbol to STT_OBJECT, but that is + incorrect because that type is for: + "A data object, such as a variable, an array, and so on". + + There is no ELF symbol to represent a compilation unit, (STT_FILE only + covers a single source file and has special sematic requirements), so + instead we use STT_NOTYPE. (Ideally we could use STT_LOOS+n, but there + is a problem with the GAS assembler, which does not allow such values to + be set on symbols). */ + fprintf (asm_out_file, "\t.type %s%s, STT_NOTYPE\n", annobin_current_filename, suffix); + + if (target_start_sym_bias) + { + /* We set the address of the start symbol to be the current address plus + a bias value. That way this symbol will not be confused for a file + start/function start symbol. + + There is special code in annobin_output_note() that undoes this bias + when the symbol's address is being used to compute a range for the + notes. */ + fprintf (asm_out_file, "\t.equiv %s%s, . + %d\n", annobin_current_filename, suffix, target_start_sym_bias); + } + else + fprintf (asm_out_file, "\t.equiv %s%s, .\n", annobin_current_filename, suffix); + + /* We explicitly set the size of the symbol to 0 so that it will not + confuse other tools (eg GDB, elfutils) which look for symbols that + cover an address range. */ + fprintf (asm_out_file, "\t.size %s%s, 0\n", annobin_current_filename, suffix); + + fprintf (asm_out_file, "\t.popsection\n"); + + const char * start = concat (annobin_current_filename, suffix, NULL); + const char * end = concat (annobin_current_endname, suffix, NULL); + const char * sec; + + if (* suffix) + sec = concat (GNU_BUILD_ATTRS_SECTION_NAME, suffix, + ", \"G\", %note, text", suffix, ".group", NULL); + else + sec = concat (GNU_BUILD_ATTRS_SECTION_NAME, suffix, NULL); + + char buffer [124]; + + sprintf (buffer, "%d%c%d", SPEC_VERSION, producer_char, annobin_version); + annobin_output_string_note (GNU_BUILD_ATTRIBUTE_VERSION, buffer, + "string: version", start, end, OPEN, sec); + + free ((void *) sec); + free ((void *) end); + free ((void *) start); +} + static void annobin_create_global_notes (void * gcc_data, void * user_data) { @@ -1137,55 +1215,15 @@ annobin_create_global_notes (void * gcc_data, void * user_data) Nevertheless we generate this symbol in the .text section as at this point we cannot know which section(s) will be used by compiled code. */ - fprintf (asm_out_file, "\t.pushsection .text\n"); - - /* Create a symbol for this compilation unit. */ - if (global_file_name_symbols) - fprintf (asm_out_file, "\t.global %s\n", annobin_current_filename); - else - fprintf (asm_out_file, "\t.hidden %s\n", annobin_current_filename); - - /* Note - we used to set the type of the symbol to STT_OBJECT, but that is - incorrect because that type is for: - "A data object, such as a variable, an array, and so on". - - There is no ELF symbol to represent a compilation unit, (STT_FILE only - covers a single source file and has special sematic requirements), so - instead we use STT_NOTYPE. (Ideally we could use STT_LOOS+n, but there - is a problem with the GAS assembler, which does not allow such values to - be set on symbols). */ - fprintf (asm_out_file, "\t.type %s, STT_NOTYPE\n", annobin_current_filename); - - if (target_start_sym_bias) - { - /* We set the address of the start symbol to be the current address plus - a bias value. That way this symbol will not be confused for a file - start/function start symbol. - - There is special code in annobin_output_note() that undoes this bias - when the symbol's address is being used to compute a range for the - notes. */ - fprintf (asm_out_file, "\t.equiv %s, . + %d\n", annobin_current_filename, - target_start_sym_bias); - } - else - fprintf (asm_out_file, "\t.equiv %s, .\n", annobin_current_filename); - - /* We explicitly set the size of the symbol to 0 so that it will not - confuse other tools (eg GDB, elfutils) which look for symbols that - cover an address range. */ - fprintf (asm_out_file, "\t.size %s, 0\n", annobin_current_filename); - - fprintf (asm_out_file, "\t.popsection\n"); - - /* Output the version of the specification supported. */ - sprintf (buffer, "%dp%d", SPEC_VERSION, annobin_version); - annobin_output_string_note (GNU_BUILD_ATTRIBUTE_VERSION, buffer, - "string: version", - annobin_current_filename, - annobin_current_endname, - OPEN, GNU_BUILD_ATTRS_SECTION_NAME); - + annobin_emit_start_sym_and_version_note ("", 'p'); + + /* GCC does not provide any way for a plugin to detect if hot/cold partitioning + will be performed on a function, and hence a .text.hot and/or .text.unlikely + section will be created. So instead we create global notes to cover these + two sections. */ + annobin_emit_start_sym_and_version_note (".hot", 'h'); + annobin_emit_start_sym_and_version_note (".unlikely", 'h'); + /* Record the version of the compiler. */ annobin_output_string_note (GNU_BUILD_ATTRIBUTE_TOOL, compiler_version, "string: build-tool", NULL, NULL, OPEN, GNU_BUILD_ATTRS_SECTION_NAME); @@ -1343,6 +1381,19 @@ annobin_create_global_notes (void * gcc_data, void * user_data) annobin_record_global_target_notes (); } +static void +annobin_emit_end_symbol (const char * suffix) +{ + fprintf (asm_out_file, "\t.pushsection .text%s\n", suffix); + fprintf (asm_out_file, "\t%s %s%s\n", + global_file_name_symbols ? ".global" : ".hidden", + annobin_current_endname, suffix); + fprintf (asm_out_file, "%s%s:\n", annobin_current_endname, suffix); + fprintf (asm_out_file, "\t.type %s%s, STT_NOTYPE\n", annobin_current_endname, suffix); + fprintf (asm_out_file, "\t.size %s%s, 0\n", annobin_current_endname, suffix); + fprintf (asm_out_file, "\t.popsection\n"); +} + static void annobin_create_loader_notes (void * gcc_data, void * user_data) { @@ -1355,15 +1406,9 @@ annobin_create_loader_notes (void * gcc_data, void * user_data) Eg because the compilation was run with the -ffunction-sections option. Nevertheless we generate this symbol because it is needed by the version note that was generated in annobin_create_global_notes(). */ - fprintf (asm_out_file, "\t.pushsection .text\n"); - if (global_file_name_symbols) - fprintf (asm_out_file, "\t.global %s\n", annobin_current_endname); - else - fprintf (asm_out_file, "\t.hidden %s\n", annobin_current_endname); - fprintf (asm_out_file, "%s:\n", annobin_current_endname); - fprintf (asm_out_file, "\t.type %s, STT_NOTYPE\n", annobin_current_endname); - fprintf (asm_out_file, "\t.size %s, 0\n", annobin_current_endname); - fprintf (asm_out_file, "\t.popsection\n"); + annobin_emit_end_symbol (""); + annobin_emit_end_symbol (".hot"); + annobin_emit_end_symbol (".unlikely"); } if (! annobin_enable_dynamic_notes) diff --git a/tests/function-sections-test b/tests/function-sections-test index ad67367..010d82f 100755 --- a/tests/function-sections-test +++ b/tests/function-sections-test @@ -60,20 +60,6 @@ $GCC -pie \ -Wl,--orphan-handling=warn \ hello.o unused_code.o hello2.o -o function-sections-test.exe -# Note - currently unused_func_1 and unused_func_2 will not be -# garbage collected. The duplicate of the linkonce_func_1 -# function however will be discarded. This is because the linker -# sees the relocations in the gnu build notes section against -# the unused functions, and since they are not in a section -# group together, it presumes that the relocations must be -# satisified. -# -# To solve this problem the annobin plugin will have to create -# a section group containing both the code and the notes. In -# fact strictly speaking GCC should be doing this, and annobin -# should just be adding the notes to the section group. - - # FIXME - we should check that the notes were parsed correctly... # FIXME - we should check for gaps. ../annocheck/annocheck --ignore-gaps function-sections-test.exe diff --git a/tests/missing-notes-test b/tests/missing-notes-test index b815a1a..d4ee833 100755 --- a/tests/missing-notes-test +++ b/tests/missing-notes-test @@ -55,7 +55,7 @@ $GCC -fplugin=$PLUGIN $OPTS $EXTRA_OPTS $srcdir/hello3.c $GCC -fplugin=$PLUGIN $OPTS $EXTRA_OPTS -shared $srcdir/hello_lib.c -o libhello.so -$GCC -L . -Wl,-z,now -Wl,-z,relro hello.o hello2.o hello3.o -lhello -o missing-notes-test.exe +$GCC -L . -pie -Wl,-z,now -Wl,-z,relro hello.o hello2.o hello3.o -lhello -o missing-notes-test.exe # FIXME - we should check that the notes were parsed correctly... ../annocheck/annocheck -v missing-notes-test.exe diff --git a/tests/unused_code.c b/tests/unused_code.c index 85c8024..c2352df 100644 --- a/tests/unused_code.c +++ b/tests/unused_code.c @@ -58,10 +58,10 @@ foo_ifunc (void) extern int bar (int); extern int baz (int) __attribute__((cold)); -void hot (int) __attribute__((optimize("-O3"),__noinline__)); +void coldy (int) __attribute__((optimize("-O3"),__noinline__)); void -hot (int x) +coldy (int x) { if (x) bar (bar (5)); @@ -70,7 +70,7 @@ hot (int x) } void -hotter (int x) +colder (int x) { if (x) bar (bar (7)); @@ -87,3 +87,16 @@ func_in_its_own_section (void) return 0; } +int disabled = -1; + +static __attribute__((constructor)) void +constructor_func (void) +{ + disabled = 1; +} + +static __attribute__((destructor)) void +destructor_func (void) +{ + disabled = 0; +} -- 2.43.5