Re: [PATCH] Document that index-cache requires build ID

[Guinevere Larsen <guinevere@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2988 bytes --]

On 4/6/25 8:15 AM, Lluís Batlle i Rossell wrote:
> Attached

Thanks for your patch! If you plan to continue contributing to GDB, we 
tend to prefer patches in the body of the email, as it makes for an 
easier time reviewing. But as your first submission, its absolutely 
fine. I've added the patch here for comments:


     From 4dfcd1888e3fc3c5707e3f697268d820679a1c7b Mon Sep 17 00:00:00 2001
    From: =?UTF-8?q?Llu=C3=ADs=20Batlle=20i=20Rossell?= <viric@viric.name>
    Date: Sun, 6 Apr 2025 13:11:06 +0200
    Subject: [PATCH] Document that index-cache requires build ID

    ---
      gdb/doc/gdb.texinfo | 2 ++
      gdb/dwarf2/read.c   | 4 ++++
      2 files changed, 6 insertions(+)

    diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
    index e034ac53295..37b65ab72ac 100644
    --- a/gdb/doc/gdb.texinfo
    +++ b/gdb/doc/gdb.texinfo
    @@ -22934,6 +22934,8 @@ Indices only work when using DWARF debugging
    information, not stabs.
      It is possible for @value{GDBN} to automatically save a copy of
    this index in a
      cache on disk and retrieve it from there when loading the same
    binary in the
      future.  This feature can be turned on with @kbd{set index-cache
    enabled on}.
    +Note that the cache will use the ELF build ID to identify the
    cached indices,
    +so files without Build ID will not have their index cached.
      The following commands can be used to tweak the behavior of the
    index cache.

Since there are documentation changes, we need Eli Zaretskii to approve 
this change. I've CC'd him in this email.


      @table @code
    diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
    index 71fd352343a..f6a221a57de 100644
    --- a/gdb/dwarf2/read.c
    +++ b/gdb/dwarf2/read.c
    @@ -2237,7 +2237,11 @@ get_gdb_index_contents_from_cache (objfile
    *obj, dwarf2_per_bfd *dwarf2_per_bfd)
      {
        const bfd_build_id *build_id = build_id_bfd_get (obj->obfd.get ());
        if (build_id == nullptr)
    +  {
    +    dwarf_read_debug_printf ("Not using index-cache for %s due to
    missing build-id",
    +            objfile_name (obj));
          return {};
    +  }

At first I thought this was better suited for an index_cache_debug 
version - since it already exists. But since we already have a dwarf 
debug printf related to index-cache, I think this should be fine.

One note for future patches, our style is to increase the indentation 
for the braces by 2 spaces, and 2 further spaces for code. And if we 
break up arguments for a function call, we align them in columns. These 
can be fixed by the maintainer who pushes your patch, though.

With the styling issues fixed, I'm happy to give my review tag, and hope 
this is approved soon!

Reviewed-By: Guinevere Larsen <guinevere@redhat.com>


        return global_index_cache.lookup_gdb_index (build_id,
    &dwarf2_per_bfd->index_cache_res);
    -- 
    2.47.0


-- 
Cheers,
Guinevere Larsen
She/Her/Hers

[-- Attachment #2: Type: text/html, Size: 4220 bytes --]