dwarf-reader: Use high bits of Bloom filter words.

Message ID 20200318102055.109824-1-gprocida@google.com
State Superseded
Headers
Series dwarf-reader: Use high bits of Bloom filter words. |

Commit Message

Giuliano Procida March 18, 2020, 10:20 a.m. UTC
  Most of the bit values used for GNU hash ELF section Bloom filtering
were being ignored due to integer narrowing, reducing missing symbol
filtering efficiency considerably.

This patch fixes this.

	* src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
	Don't narrow calculated Bloom word to 8 bits before using it
	to mask the fetched Bloom word.
---
 src/abg-dwarf-reader.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Matthias Männich March 18, 2020, 11:37 a.m. UTC | #1
Hi!

On Wed, Mar 18, 2020 at 10:20:55AM +0000, Android Kernel Team wrote:
>Most of the bit values used for GNU hash ELF section Bloom filtering
>were being ignored due to integer narrowing, reducing missing symbol
>filtering efficiency considerably.

Nice! Maybe add the analysis result from the other mail thread to point
out the improvement made here (and the justification for this change).

>
>This patch fixes this.
>
>	* src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
>	Don't narrow calculated Bloom word to 8 bits before using it
>	to mask the fetched Bloom word.
Please sign off your work.

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> src/abg-dwarf-reader.cc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index 3454fcf5..5556bde5 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -2025,7 +2025,7 @@ lookup_symbol_from_gnu_hash_tab(const environment*		env,
>   // filter, in bits.
>   int c = get_elf_class_size_in_bytes(elf_handle) * 8;
>   int n =  (h1 / c) % ht.bf_nwords;
>-  unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
>+  GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
>
>   // Test if the symbol is *NOT* present in this ELF file.
>   if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)
>-- 
>2.25.1.481.gfbce0eb801-goog
>
>
  
Giuliano Procida March 18, 2020, 12:13 p.m. UTC | #2
Will do.

I'll also change the commit name s/high/all/.

Giuliano.

On Wed, 18 Mar 2020 at 11:37, Matthias Maennich <maennich@google.com> wrote:
>
> Hi!
>
> On Wed, Mar 18, 2020 at 10:20:55AM +0000, Android Kernel Team wrote:
> >Most of the bit values used for GNU hash ELF section Bloom filtering
> >were being ignored due to integer narrowing, reducing missing symbol
> >filtering efficiency considerably.
>
> Nice! Maybe add the analysis result from the other mail thread to point
> out the improvement made here (and the justification for this change).
>
> >
> >This patch fixes this.
> >
> >       * src/abg-dwarf-reader.cc (lookup_symbol_from_gnu_hash_tab):
> >       Don't narrow calculated Bloom word to 8 bits before using it
> >       to mask the fetched Bloom word.
> Please sign off your work.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >---
> > src/abg-dwarf-reader.cc | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> >index 3454fcf5..5556bde5 100644
> >--- a/src/abg-dwarf-reader.cc
> >+++ b/src/abg-dwarf-reader.cc
> >@@ -2025,7 +2025,7 @@ lookup_symbol_from_gnu_hash_tab(const environment*               env,
> >   // filter, in bits.
> >   int c = get_elf_class_size_in_bytes(elf_handle) * 8;
> >   int n =  (h1 / c) % ht.bf_nwords;
> >-  unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
> >+  GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
> >
> >   // Test if the symbol is *NOT* present in this ELF file.
> >   if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)
> >--
> >2.25.1.481.gfbce0eb801-goog
> >
> >
  

Patch

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 3454fcf5..5556bde5 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -2025,7 +2025,7 @@  lookup_symbol_from_gnu_hash_tab(const environment*		env,
   // filter, in bits.
   int c = get_elf_class_size_in_bytes(elf_handle) * 8;
   int n =  (h1 / c) % ht.bf_nwords;
-  unsigned char bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
+  GElf_Word bitmask = (1ul << (h1 % c)) | (1ul << (h2 % c));
 
   // Test if the symbol is *NOT* present in this ELF file.
   if ((bloom_word_at(elf_handle, ht.bloom_filter, n) & bitmask) != bitmask)