From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2591 invoked by alias); 17 Aug 2015 22:45:09 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2579 invoked by uid 89); 17 Aug 2015 22:45:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f201.google.com Received: from mail-pd0-f201.google.com (HELO mail-pd0-f201.google.com) (209.85.192.201) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Mon, 17 Aug 2015 22:45:08 +0000 Received: by pdbfa8 with SMTP id fa8so14146607pdb.1 for ; Mon, 17 Aug 2015 15:45:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:message-id:date:subject:from:to:cc :content-type; bh=4J5ciLYicJY8OxkEN2UHdkRwEENjPNKnZu8RsRduP38=; b=l0lCX6sx7sGjhWXW3ZldaC1322bLqOcYw2JumFcmyWZJim21BzsXnY+C5imw5hjv3X vOAFg0FAuKsDI7kRHmgqgHLDIbUvNLGQ9iWYa/h+EX3UnVup9aoHu4xwy4+iTslH3qpa KgkKtK/4hoiheD+RO8nkkVCldbOy4pVe+52p3+SnF5swmliwSIW61jCNaeDjGTuHj+UO OEIqNHSA+nPDt4s4vbll1gFso3fUBf8mbFSQhwiWx7js+8kLErmQZg8JHC3BMoEXZ/+S +vJSbFVfMaXSKsD6zdGo//LoLzcDIboGdiHj2w8pdpNv1Xzl23vbRzF3y3pG1qz7fK90 QkeA== X-Gm-Message-State: ALoCoQl45lwZgwJhhBSBJcupZ0uoGDpukYCQCcN0UoL1Bw754yrtvZ5v46j6ueSZ+qf+Q43J2YJC MIME-Version: 1.0 X-Received: by 10.67.14.231 with SMTP id fj7mr3205754pad.33.1439851505978; Mon, 17 Aug 2015 15:45:05 -0700 (PDT) Message-ID: <001a113453ec8b4fc0051d898d51@google.com> Date: Mon, 17 Aug 2015 22:45:00 -0000 Subject: Re: [PATCH] dwarf2read.c: Check type of linkage name attribute prior to decoding From: Doug Evans To: Kevin Buettner Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=UTF-8; format=flowed; delsp=yes X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00447.txt.bz2 Kevin Buettner writes: > On Mon, 3 Aug 2015 16:31:08 -0700 > Doug Evans wrote: > > > I wonder, though, if this is a good place for using the dwarf assembler. > > Seems so. We just need a MIPS_linkage_name attribute > > that isn't a string. The dwarf assembler test would be a lot > > smaller. > > My updated patch, below, does this. > > > What if there was a wrapper on dwarf2_attr, dwarf2_string_attr > > or some such, and it returned either the attribute (if the attribute > > is present *and* is a string) or NULL. > > And if the attribute is present but not a string it logs a > > complaint (standard bad debug info complaint) and returns NULL. > > I've introduced the wrapper that you recommend and have used it in > all places that made sense to me. There were a few spots where using > it would have made things more complicated, so I left those alone. > > Here's the updated change / patch: > > dwarf2read.c: Check type of string valued attributes prior to decoding. > > This change introduces a new function, dwarf2_string_attr(), which is > a wrapper for dwarf2_attr(). dwarf2read.c has been updated to > call dwarf2_string_attr in most instances where a string-valued > attribute is decoded to produce a string value. In most cases, it > simplifies the code; in some instances, the complexity of the code > remains unchanged. > > I performed this change by looking for instances where the > result of DW_STRING was used in an assignment. Many of these > had a pattern which (roughly) looks something like this: > > struct attribute *attr = NULL; > > attr = dwarf2_attr (die, name, cu); > if (attr != NULL && DW_STRING (attr)) > { > const char *str; > ... > str = DW_STRING (attr); > ... /* Use str in some fashion. */ > } > > Code of this form is transformed to look like this instead: > > const char *str; > > str = dwarf2_string_attr (die, name, cu) > if (str != NULL) > { > ... > /* Use str in some fashion. */ > ... > } > > In addition to invoking dwarf2_attr() and DW_STRING(), > dwarf2_string_attr() checks to make sure that the attribute's > `form' field matches one of DW_FORM_strp, DW_FORM_string, or > DW_FORM_GNU_strp_alt. If it does not match one of these forms, > it will return a NULL value in addition to calling complaint(). > > An earlier version of this patch did this type checking for one > particular instance where a string attribute was being decoded. > The situation that I was attempting to handle in that earlier patch is > this: > > The Texas Instruments compiler uses the encoding for > DW_AT_MIPS_linkage_name for other purposes. TI uses the encoding, > 0x2007, for TI_AT_TI_end_line which, unlike DW_AT_MIPS_linkage_name, > does not have a string-typed value. In this instance, GDB was attempting > to use an integer value as a string pointer, with predictable results. > (GDB would die with a segmentation fault.) > > I've added a test which reproduces the problem that I was orignally > wanting to fix. It uses DW_AT_MIPS_linkage name with an associate > value which is a string, and again, where the value is a small > integer. > > My test case causes GDB to segfault in an unpatched GDB. There > will be two PASSes in a patched GDB. > > Unpatched GDB: > > (gdb) ptype f > ERROR: Process no longer exists > UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f > ERROR: Couldn't send ptype g to GDB. > UNRESOLVED: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g > > Patched GDB: > > (gdb) ptype f > type = bool () > (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype f > ptype g > type = bool () > (gdb) PASS: gdb.dwarf2/dw2-bad-mips-linkage-name.exp: ptype g > > I see no regressions on an x86_64 native target. > > gdb/ChangeLog: > > * dwarf2read.c (dwarf2_string_attr): New function. > (lookup_dwo_unit, process_psymtab_comp_unit_reader) > (dwarf2_compute_name, dwarf2_physname, find_file_and_directory) > (read_call_site_scope, namespace_name, guess_full_die_structure_name) > (anonymous_struct_prefix, prepare_one_comp_unit): Use > dwarf2_string_attr in place of dwarf2_attr and DW_STRING. > > gdb/testsuite/ChangeLog: > > * gdb.dwarf2/dw2-bad-mips-linkage-name.c: New file. > * gdb.dwarf2/dw2-bad-mips-linkage-name.exp: New file. Thanks for the ping! LGTM