From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 106980 invoked by alias); 6 Sep 2019 17:11:59 -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 106971 invoked by uid 89); 6 Sep 2019 17:11:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS autolearn=ham version=3.3.1 spammy= X-HELO: aserp2120.oracle.com Received: from aserp2120.oracle.com (HELO aserp2120.oracle.com) (141.146.126.78) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 06 Sep 2019 17:11:58 +0000 Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x86H8qvE171854; Fri, 6 Sep 2019 17:11:42 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2019-08-05; bh=Jl5LdAno/YyMKsNRhd6nY8z1RAnYBzLHqcKWqJtnzBA=; b=mb7VBxFzBLaV8Gv+MehiO3q915Aeg/Qz4Qa5BYUhLCK2wEG7y0p1ZgPoyGEY0qZB52XD VUY+zqNDLIeeWjr3v5PyZP6LMdK4OrzrPZ26Tj/HmNRe/T9Z50mtCsHbevVUj+eJgiZD sCMnnSonUjJUP3Hj0ndi5uf3tdd3rjEYGy1CyjHwxU4skInifqemUttp+0HzbQsQMglb JO4gB/Fatjz77juNc39gxWWg98VYh8FC+My6GWOtM4PVeFxb0/Vp3JtH6ejrUqUh2afx O1MU7NRcGHbBfih9+ejK4mv2HjmQYuhWiOfVZbHhz3KiEPQh6AI7X4FI4i0gmsYSzpx4 Bw== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by aserp2120.oracle.com with ESMTP id 2uuufu01sp-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 06 Sep 2019 17:11:42 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id x86H4WQ5124433; Fri, 6 Sep 2019 17:11:41 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userp3020.oracle.com with ESMTP id 2uum4hbmfa-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 06 Sep 2019 17:11:41 +0000 Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id x86HBcNd003519; Fri, 6 Sep 2019 17:11:40 GMT Received: from [10.175.31.40] (/10.175.31.40) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 06 Sep 2019 10:11:38 -0700 Subject: Re: Fix a crash in compile_to_object function To: Keith Seitz , gdb-patches@sourceware.org References: <3ad99625-43ff-a450-6e50-d83242224443@oracle.com> <1742a18e-dfcb-6d80-03cc-5696a0cd3cd3@redhat.com> From: libor.bukata@oracle.com Message-ID: Date: Fri, 06 Sep 2019 17:11:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <1742a18e-dfcb-6d80-03cc-5696a0cd3cd3@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00069.txt.bz2 Hi Keith, I have no objections to use 'os_rx != NULL ? os_rx : ""' instead; it is more portable. Note that NULL should be used instead of nullptr as nullptr is available since C++11. Thanks, Libor feel free to use 'os_rx != nullptr ? os_rx : ""' instead On 9/6/19 6:42 PM, Keith Seitz wrote: > On 9/6/19 6:37 AM, libor.bukata@oracle.com wrote: >> On non-Linux platforms, gdb crashes when compile command is issued >> because of the null pointer in struct osabi_names gdb_osab. The attached >> patch adds a check to avoid this crash and adds osabi name for Solaris. >> However, there is probably more work required to enable compile feature >> on Solaris (e.g., solaris_infcall_munmap) and other platforms. >> > Thank you for the patch! The compile feature, as you have discovered, needs > much more testing on non-linux configurations. > >> --- gdb-8.3/gdb/compile/compile.c 2019-08-19 13:07:57.669785758 +0000 >> +++ gdb-8.3/gdb/compile/compile.c 2019-08-19 13:07:33.865626973 +0000 >> @@ -697,7 +697,7 @@ compile_to_object (struct command_line * >> const char *arch_rx = gdbarch_gnu_triplet_regexp (gdbarch); >> >> /* Allow triplets with or without vendor set. */ >> - triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + os_rx; >> + triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + (os_rx ? : ""); >> compiler->set_triplet_regexp (triplet_rx.c_str ()); >> } > I'm not sure about this. Should os_rx be NULL (which you've shown is possible > for a number of configurations), this would leave triplet_rx as $ARCH(-[^-]*)?-", > e.g., "x86_64(-[^-]*)?-". I would call that a malformed regexp for this purpose. > > While the plugin may handle that gracefully, this just doesn't seem very > user-friendly to me, but I am not a maintainer, so you should definitely > wait for a real maintainer to chime in. > > Otherwise, the only comments I have relate to > >> + triplet_rx = std::string (arch_rx) + "(-[^-]*)?-" + (os_rx ? : ""); > We prefer explicit comparisons with NULL/nullptr. > > Omitting the true-case expression is unusual. Not invalid, but certainly > unusual (in gdb). I find no uses of this idiom in our sources. I would > prefer to see the more explicit > > os_rx != nullptr ? os_rx : "" > > but I'll let official maintainers chime in on this usage. I'm just as curious > to see how others feel about it. > > Keith