Re: [PATCH] dwarf-reader: Use high bits of Bloom filter words.

[Giuliano Procida <gprocida@google.com>]

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
> >
> >