Re: Fix a crash in compile_to_object function

[libor.bukata@oracle.com]

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