public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* Patch - Crash while printing static variables without global symbols.
@ 2015-02-25 21:25 Sławomir Wojtasiak
  2015-02-26 11:02 ` Yao Qi
  0 siblings, 1 reply; 3+ messages in thread
From: Sławomir Wojtasiak @ 2015-02-25 21:25 UTC (permalink / raw)
  To: gdb-patches

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

Hello,

I've found a bug related to the variable printing functionality for C++
language. The bug showed itself when GDB tried to print a static
variable and global symbol describing that variable wasn't found in
object files. I've added a simple sanity checking just to prevent
accessing an empty pointer in such a case, by skipping
'cp_print_static_field' (gdb/cp-varprint.c:329) function.

Attached archive consists of the following files:
0001-This-fix-prevents-crashes-while-accessing-static-var.patch - commit
as git format-path

0001-This-fix-prevents-crashes-while-accessing-static-var.diff - plain
diff.

ChangeLog - Change log entry.

Regards,
SÅ‚awomir Wojtasiak


[-- Attachment #2: gdb-patch.tgz --]
[-- Type: application/x-compressed-tar, Size: 990 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Patch - Crash while printing static variables without global symbols.
  2015-02-25 21:25 Patch - Crash while printing static variables without global symbols Sławomir Wojtasiak
@ 2015-02-26 11:02 ` Yao Qi
  2015-02-28  0:04   ` Slawomir Wojtasiak
  0 siblings, 1 reply; 3+ messages in thread
From: Yao Qi @ 2015-02-26 11:02 UTC (permalink / raw)
  To: Sławomir Wojtasiak; +Cc: gdb-patches

Sławomir Wojtasiak <slawomir.wojtasiak@fcml-lib.com> writes:

> I've found a bug related to the variable printing functionality for C++
> language. The bug showed itself when GDB tried to print a static
> variable and global symbol describing that variable wasn't found in
> object files. I've added a simple sanity checking just to prevent
> accessing an empty pointer in such a case, by skipping
> 'cp_print_static_field' (gdb/cp-varprint.c:329) function.

Thanks for your patch, however the way you post your patch isn't
friendly to people to review it.  You can use 'git send-email'
to send your patch to gdb-patches@, instead of sending a .tgz, so that
people can read your patch in their mailer, and review it easily.

Could you please provide a reproducer, a cpp file, and steps cause gdb
crash, and after your patch, the crash disappeared?  We can convert it
to a dejagnu test case for gdb later.

>
> Attached archive consists of the following files:
> 0001-This-fix-prevents-crashes-while-accessing-static-var.patch - commit
> as git format-path
>
> 0001-This-fix-prevents-crashes-while-accessing-static-var.diff - plain
> diff.
>
> ChangeLog - Change log entry.

We include changelog entry in the git commit message too, please take a
look at https://sourceware.org/gdb/wiki/ContributionChecklist

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Patch - Crash while printing static variables without global symbols.
  2015-02-26 11:02 ` Yao Qi
@ 2015-02-28  0:04   ` Slawomir Wojtasiak
  0 siblings, 0 replies; 3+ messages in thread
From: Slawomir Wojtasiak @ 2015-02-28  0:04 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On Thu, 2015-02-26 at 11:02 +0000, Yao Qi wrote:
> 
> > I've found a bug related to the variable printing functionality for C++
> > language. The bug showed itself when GDB tried to print a static
> > variable and global symbol describing that variable wasn't found in
> > object files. I've added a simple sanity checking just to prevent
> > accessing an empty pointer in such a case, by skipping
> > 'cp_print_static_field' (gdb/cp-varprint.c:329) function.
> 
> Thanks for your patch, however the way you post your patch isn't
> friendly to people to review it.  You can use 'git send-email'
> to send your patch to gdb-patches@, instead of sending a .tgz, so that
> people can read your patch in their mailer, and review it easily.
> 
> Could you please provide a reproducer, a cpp file, and steps cause gdb
> crash, and after your patch, the crash disappeared?  We can convert it
> to a dejagnu test case for gdb later.

Thanks for the information. Next time I will try to prepare everything
the way you've mentioned. Anyway, in order to provide reproducer I will
need a bit more time to analyse the source of the problem in depth. It's
probably related to the way GCC encodes static constants in DWARF2
sections. It seems that everything is going well as long as
DW_AT_const_value attribute is used directly inside DW_TAG_member to
store the constant's value. For instance:

<2><4c7d>: Abbrev Number: 88 (DW_TAG_member)
    <4c7e>   DW_AT_name        : (indirect string, offset: 0x5256):
size1
    <4c82>   DW_AT_decl_file   : 4
    <4c83>   DW_AT_decl_line   : 19
    <4c84>   DW_AT_type        : <0x4c8a>
    <4c88>   DW_AT_external    : 1
    <4c88>   DW_AT_accessibility: 1 (public)
    <4c89>   DW_AT_declaration : 1
    <4c89>   DW_AT_const_value : -1

The issue shows itself when GCC links static constant to a symbol in the
object file using DW_AT_linkage_name, but the symbol is stripped out
from the object file probably as unused one. In such a case the debugger
is not able to get the value of the static constant in order to print it
and it returns an empty pointer, which is then accessed and kaboom!

The problem is that there is probably no way to control how such a value
is encoded inside DWARF2 on the source code level. It's why the
reproducer in a form of plain CPP file is probably not an option...

Other than that, trying to reproduce the problem on two different
machines I've observed that the same static constant can be encoded
differently with no concrete reason (the same code base), so I wasn't
able to reproduce the problem on another computer. Anyway, I will take a
look at it once again in my spare time, just to be sure.

> >
> > Attached archive consists of the following files:
> > 0001-This-fix-prevents-crashes-while-accessing-static-var.patch - commit
> > as git format-path
> >
> > 0001-This-fix-prevents-crashes-while-accessing-static-var.diff - plain
> > diff.
> >
> > ChangeLog - Change log entry.
> 
> We include changelog entry in the git commit message too, please take a
> look at https://sourceware.org/gdb/wiki/ContributionChecklist
> 

Thanks for the link I will read it over carefully.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-02-28  0:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-25 21:25 Patch - Crash while printing static variables without global symbols Sławomir Wojtasiak
2015-02-26 11:02 ` Yao Qi
2015-02-28  0:04   ` Slawomir Wojtasiak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).