Fix a crash in compile_to_object function

Keith Seitz keiths@redhat.com
Fri Sep 6 16:42:00 GMT 2019


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



More information about the Gdb-patches mailing list