Fix warning: Invalid entry in .debug_gdb_scripts section

Message ID 20170316173913.GA5173@host1.jankratochvil.net
State New, archived
Headers

Commit Message

Jan Kratochvil March 16, 2017, 5:39 p.m. UTC
  On Thu, 16 Mar 2017 18:03:41 +0100, Pedro Alves wrote:
> I don't think you've supplied enough rationale.

I forgot to copy-paste prepared:

/usr/bin/rustc
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [15] .debug_gdb_scripts PROGBITS        00000000000008ed 0008ed 000022 00 AMS  0   0  1

/usr/lib/debug/usr/bin/rustc.debug
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [15] .debug_gdb_scripts NOBITS          00000000000008ed 000280 000022 00 AMS  0   0  1


> Shouldn't we be able to load scripts from debug-only sections?

I agree there is better to use SEC_HAS_CONTENTS, right?


> A testcase would indeed be good to have.

OK, I will update the patch later.


Jan
gdb/ChangeLog
2017-03-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* auto-load.c (auto_load_section_scripts): Check SEC_HAS_CONTENTS.
  

Comments

Pedro Alves March 16, 2017, 6:02 p.m. UTC | #1
On 03/16/2017 05:39 PM, Jan Kratochvil wrote:
> On Thu, 16 Mar 2017 18:03:41 +0100, Pedro Alves wrote:
>> I don't think you've supplied enough rationale.
> 
> I forgot to copy-paste prepared:
> 
> /usr/bin/rustc
> Section Headers:
>   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>   [15] .debug_gdb_scripts PROGBITS        00000000000008ed 0008ed 000022 00 AMS  0   0  1
> 
> /usr/lib/debug/usr/bin/rustc.debug
> Section Headers:
>   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
>   [15] .debug_gdb_scripts NOBITS          00000000000008ed 000280 000022 00 AMS  0   0  1
> 

Well, that's just a tool output, not a self-contained
rationale/explanation.  With only that, any reviewer will have to
guess things that you probably have already discovered while
debugging the issue.  Josh said that the script is still loaded
fine.  I guess that's because GDB actually tries to read the script
twice, once from rustc, which succeeds, and another time from
rustc.debug, which runs into a zero'ed script ID type, which is
invalid exactly to catch this type of problems.

>> Shouldn't we be able to load scripts from debug-only sections?
> 
> I agree there is better to use SEC_HAS_CONTENTS, right?

Yes, I think so.

Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 56914c8..edaf264 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1174,7 +1174,8 @@  auto_load_section_scripts (struct objfile *objfile, const char *section_name)
   bfd_byte *data = NULL;
 
   scripts_sect = bfd_get_section_by_name (abfd, section_name);
-  if (scripts_sect == NULL)
+  if (scripts_sect == NULL
+      || (bfd_get_section_flags (abfd, scripts_sect) & SEC_HAS_CONTENTS) == 0)
     return;
 
   if (!bfd_get_full_section_contents (abfd, scripts_sect, &data))