This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: gold patch for fixing ICF bugs.


> ? ? ? ?* icf.cc (get_section_contents): Check for preemptible functions.
> ? ? ? ?Ignore addend when appropriate.
> ? ? ? ?* symtab.cc (should_add_dynsym_entry): Add new parameter. ?Check for
> ? ? ? ?section folded.
> ? ? ? ?(add_from_relobj): Check for section folded.
> ? ? ? ?(set_dynsym_indexes): Fix call to should_add_dynsym_entry.
> ? ? ? ?* symtab.h (should_add_dynsym_entry): Add new parameter.
> ? ? ? ?* target-reloc.h (scan_relocs): Check for section folded.
> ? ? ? ?* x86_64.cc (Target_x86_64::Scan::possible_function_pointer_reloc):
> ? ? ? ?Check reloc types for function pointers in shared objects.
> ? ? ? ?* testsuite/Makefile.am (icf_virtual_function_folding_test): New test
> ? ? ? ?case.
> ? ? ? ?(icf_preemptible_functions_test): New test case.
> ? ? ? ?(icf_string_merge_test): New test case.
> ? ? ? ?* testsuite.Makefile.in: Regenerate.
> ? ? ? ?* testsuite/icf_safe_so_test.sh: Change to not fold foo_glob and
> ? ? ? ?bar_glob. ?Refactor code.
> ? ? ? ?* testsuite/icf_preemptible_functions_test.cc: New file.
> ? ? ? ?* testsuite/icf_preemptible_functions_test.sh: New file.
> ? ? ? ?* testsuite/icf_string_merge_test.cc: New file.
> ? ? ? ?* testsuite/icf_string_merge_test.sh: New file.
> ? ? ? ?* testsuite/icf_virtual_function_folding_test.cc: New file.
> ? ? ? ?* testsuite/icf_virtual_function_folding_test.sh: New file.

It looks good to me; I just have a nit on one comment:

+		  long long offset = it_a->first;
+
+                  unsigned long long addend = it_a->second;
+                  // Ignoring the addend when it is a negative value --
+                  // this is taken from function 'Value' in file 'object.h'.
+                  if (addend < 0xffffff00)
+                    offset = offset + it_a->second;

The comment should be a complete sentence: "Ignore the addend when
...". Instead of saying "this is taken from", it would be better to
say "See the comments for Merged_symbol_value::value() in object.h."

-cary


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]