Project

General

Profile

Bug #10817

ctfconvert -i option is mis-handled

Added by John Levon 7 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-04-18
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

Description

http://smartos.org/bugview/OS-7639

This issue is a combination of problems specific to the new CTF tools, and existing dubious behavior.

I noticed that in a Makefile that was using ctfconvert -i on a .so file, I was not getting CTF out of the end - silently. If I didn't specify -i, the same happened. The first bug is in the new tools:

134 ctf_convert_ftypes(elf, &type);
135 printf("got types: d\n", type);
136 if (flags x%x
CTF_CONVERT_F_IGNNONC) {
137 if (type == CTFCONV_SOURCE_NONE ||
138 (type & CTFCONV_SOURCE_UNKNOWN)) {
139 *errp = ECTF_CONVNOCSRC;
140 return (NULL);
141 }
142 }

The check on line :136 is reversed: we should be reporting an error if -i is not specified. As it happens it always is under illumos so we probably didn't notice this.

The second issue is when we use ctfconvert in "merge" mode as we are in this case:

.../ctfconvert -L VERSION -i sysinfo_mod.so

In this mode, the -i option really doesn't make any sense as it's currently implemented. We're going to see STT_FILE entries for sysinfo_mod.so, consider it CTFCONV_SOURCE_UNKNOWN, and we would fail with an error if the above bug was fixed and we didn't specify -i. That is, it's always required.

The original code made a little more sense when we were only converted one .o file at a time: there, we typically only see the one source file foo.c

But this does make me question what the semantics of this check really should be. What bug or issue are we trying to protect against with this code?

History

#1

Updated by Electric Monk 6 months ago

  • % Done changed from 0 to 100
  • Status changed from New to Closed

git commit 3eca610387779e26c8c63e26d2ba418b0cd1bf5a

commit  3eca610387779e26c8c63e26d2ba418b0cd1bf5a
Author: John Levon <john.levon@joyent.com>
Date:   2019-05-03T09:15:58.000Z

    10816 ctf_dwarf_convert_type() relies on un-initialized id
    10817 ctfconvert -i option is mis-handled
    10818 Improve ctfconvert error messages
    10819 ctfconvert should handle empty dies
    10820 ctfconvert -i never converts
    10821 bad free in ctf_dwarf_init_die
    10815 shouldn't build gcore.c as part of kmdb
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Approved by: Gordon Ross <gwr@nexenta.com>

Also available in: Atom PDF