On Dec 22, 2024 10:12, Simon Marchi<simon.marchi@polymtl.ca> wrote:
> On 2024-12-21 03:30, Yang Liu wrote:
> > The custom linetable functionality in GDB's JIT Interface has been broken
> > since commit 1acc9dca423f78e44553928f0de839b618c13766.
> >
> > In that commit, linetables were made independent from the objfile, which
> > requires objfile->section_offsets to be initialized. However, section_offsets
> > were never initialized in objfiles generated by GDB's JIT Interface
> > with custom jit-readers, leading to GDB crashes when stepping into JITed code
> > blocks with the following command already executed:
> >
> > jit-reader-load libmygdbjitreader.so
> >
> > This patch fixes the issue by initializing the minimum section_offsets required
> > for linetable parsing procedures.
>
> I can imagine how the problem can manifest, but do you have a
> reproducer? I tried stepping with the binary of test
> gdb.base/jit-reader.exp, but I couldn't get it to crash. Ideally, this
> fix should come with a test to demonstrate the problem and the fix.
>


Hi Simon,


Thanks for the review!


Below is a reproducer, but I have no idea how GDB testsuite works, is it possible
to put this in?


// Compile with:
gcc -ggdb -o jit jit.c
gcc -shared -fPIC -o libmyreader.so reader.c


// Run with:
gdb --args ./jit
(gdb) jit-reader-load /path/to/libmyreader.so
(gdb) b jit.c:90
(gdb) step <-- GDB crashes here


// Source files:
// jit.h ------------------------------
#include <stdio.h>
#include <stdint.h>
#include <gdb/jit-reader.h>


typedef struct {
char filename[32];
FILE* file;
GDB_CORE_ADDR start;
GDB_CORE_ADDR end;
size_t nlines;
struct gdb_line_mapping lines[3];
} gdbjit_block_t;


// jit.c ------------------------------
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <sys/mman.h>
#include "jit.h"


enum {
GDBJIT_NOACTION = 0,
GDBJIT_REGISTER,
GDBJIT_UNREGISTER
};


typedef struct gdbjit_code_entry_s {
struct gdbjit_code_entry_s* next_entry;
struct gdbjit_code_entry_s* prev_entry;
const char* symfile_addr;
uint64_t symfile_size;
} gdbjit_code_entry_t;


typedef struct gdbjit_descriptor_s {
uint32_t version;
uint32_t action_flag;
gdbjit_code_entry_t* relevant_entry;
gdbjit_code_entry_t* first_entry;
} gdbjit_descriptor_t;


void __attribute__((noinline, visibility("default"))) __jit_debug_register_code()
{
asm volatile("" ::: "memory");
};


__attribute__((visibility("default")))
gdbjit_descriptor_t __jit_debug_descriptor = { 1, GDBJIT_NOACTION, NULL, NULL };


static void gdbjit_block_ready(GDB_CORE_ADDR start, GDB_CORE_ADDR end) {
gdbjit_block_t* block = (gdbjit_block_t*)calloc(sizeof(gdbjit_block_t), 1);
strcpy(block->filename, "/tmp/mygdbjit-XXXXXX.S");
int fd = mkstemps(block->filename, 2);
block->file = fdopen(fd, "w");
block->start = start;
block->end = end;
fprintf(block->file, "mov eax, edi\nadd eax, esi\nret\n");
fclose(block->file);
block->nlines = 3;
block->lines[0].pc = start;
block->lines[0].line = 1;
block->lines[1].pc = start + 2;
block->lines[1].line = 2;
block->lines[2].pc = start + 4;
block->lines[2].line = 3;
gdbjit_code_entry_t* entry = (gdbjit_code_entry_t*)malloc(sizeof(gdbjit_code_entry_t));
entry->symfile_addr = (const char*)block;
entry->symfile_size = sizeof(gdbjit_block_t);


if (__jit_debug_descriptor.first_entry) {
__jit_debug_descriptor.relevant_entry->next_entry = entry;
entry->prev_entry = __jit_debug_descriptor.relevant_entry;
} else {
__jit_debug_descriptor.first_entry = entry;
}


__jit_debug_descriptor.relevant_entry = entry;
__jit_debug_descriptor.action_flag = GDBJIT_REGISTER;
__jit_debug_register_code();
}


typedef int (*func_ptr)(int, int);


int main() {
unsigned char code[] = {
0x89, 0xF8, // mov eax, edi
0x01, 0xF0, // add eax, esi
0xC3 // ret
};


void *mem = mmap(NULL, sizeof(code), PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
if (mem == MAP_FAILED) {
perror("mmap");
return EXIT_FAILURE;
}


memcpy(mem, code, sizeof(code));
gdbjit_block_ready((GDB_CORE_ADDR)mem, (GDB_CORE_ADDR)mem + sizeof(code));


func_ptr add = (func_ptr)mem;


int a = 5, b = 7;
int result = add(a, b);
printf("Result of %d + %d = %d\n", a, b, result);
munmap(mem, sizeof(code));
return 0;
}


// reader.c ------------------------------
#include "jit.h"
#include <stdio.h>


static enum gdb_status read_debug_info(struct gdb_reader_funcs* self,
struct gdb_symbol_callbacks* cbs, void* memory, long memory_sz)
{
gdbjit_block_t* block = (gdbjit_block_t*)memory;


struct gdb_object* object = cbs->object_open(cbs);
struct gdb_symtab* symtab = cbs->symtab_open(cbs, object, block->filename);
cbs->block_open(cbs, symtab, NULL, block->start, block->end, "(block)");


cbs->line_mapping_add(cbs, symtab, block->nlines, block->lines);


cbs->symtab_close(cbs, symtab);
cbs->object_close(cbs, object);


return GDB_SUCCESS;
}


enum gdb_status unwind_frame(struct gdb_reader_funcs* self, struct gdb_unwind_callbacks* cbs)
{
return GDB_SUCCESS;
}


struct gdb_frame_id get_frame_id(struct gdb_reader_funcs* self, struct gdb_unwind_callbacks* cbs)
{
struct gdb_frame_id frame = {0xdeadbeef, 0};
return frame;
}


void destroy_reader(struct gdb_reader_funcs* self) {}


__attribute__((visibility("default"))) struct gdb_reader_funcs* gdb_init_reader(void)
{
static struct gdb_reader_funcs funcs = {GDB_READER_INTERFACE_VERSION, NULL,
read_debug_info, unwind_frame, get_frame_id, destroy_reader};
return &funcs;
}


__attribute__((visibility("default"))) int plugin_is_GPL_compatible (void)
{
return 0;
}


> > ---
> > gdb/jit.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/gdb/jit.c b/gdb/jit.c
> > index 77d41bf86ba..7027c84b5bf 100644
> > --- a/gdb/jit.c
> > +++ b/gdb/jit.c
> > @@ -665,6 +665,8 @@ jit_object_close_impl (struct gdb_symbol_callbacks *cb,
> >
> > objfile *objfile = objfile::make (nullptr, current_program_space,
> > objfile_name.c_str (), OBJF_NOT_FILENAME);
> > + objfile->section_offsets.assign (1, 0);
>
> I would suggest using push_back instead of assign. Everybody knows
> push_back, whereas I had to look up what assign does, I guess I'm not
> the only one.
>


Thank you for the advice, I will address this in V2.


Reagrds,
Yang Liu


> Simon