undoio/l3

Logic in l3_dump.py is buggy and incomplete. Does not handle large #s of trace records and EOF cleanly.

Closed this issue · 2 comments

This issue was discovered while browsing through the code of l3_dump.py and in an attempt to unit-test its functionality.

The dump script is written in a bit of a hard-coded way, expecting only some # of log-entries. See this:

 37     for _ in range(3):
 38         row = file.read(32)
 39         tid, loc, ptr, arg1, arg2 = struct.unpack('<iiQQQ', row)
 40         offs = ptr - fibase - rodata_offs
 41         if loc == 0:
 42             print(f"{tid=} '{strings[offs]}' {arg1=} {arg2=}")
 43         else:
 44             print(f"{tid=} {loc=} '{strings[offs]}' {arg1=} {arg2=}")

Consequently for the use-case sample programs we exercise thru CI, only 3 log-entries are printed.

A simple attempt to write a stand-alone exerciser of L3 log APIs to generate more entries (4) shows that only the 1st 3 are decoded and printed. See sample outputs below.

Proposal:

  • Tighten the logic in this dump script to work with any number of entries
  • Add logic to only parse "known" L3-generated log-files (Magic indicator?)
  • Tighten the dependencies of L3 struct-definitions that appear in this Py script as hard-coded constants 32

Demo to show the incomplete behaviour.

[1] is a simple l3_dump.py-test.c, which logs 4 entries, as:

 28     l3_log_simple("Simple-log-msg-Args(1,2)", 1, 2);
 29     l3_log_simple("Simple-log-msg-Args(3,4)", 3, 4);
 30     l3_log_simple("Potential memory overwrite (addr, size)", 0xdeadbabe, 1024);
 31     l3_log_simple("Invalid buffer handle (addr)", 0xbeefabcd, 0);

Upon running this exercise, we [expectedly] only see the 1st 3 entries.

Build the test program and exercise it:

$ g++ -I ./include -o l3_dump.py-test l3_dump.py-test.c src/l3.c l3.S

agurajada-Linux-Vm:[59] $ g++ -I ./include -o l3_dump.py-test l3_dump.py-test.c src/l3.c l3.S
agurajada-Linux-Vm:[60] $ ./l3_dump.py-test

agurajada-Linux-Vm:[61] $ ll /tmp/l3.c-small-unit-test.dat
-rw-rw-r-- 1 agurajada 524321 Mar 28 05:21 /tmp/l3.c-small-unit-test.dat

Run the tool to extract the entries:

agurajada-Linux-Vm:[66] $ ./l3_dump.py /tmp/l3.c-small-unit-test.dat ./l3_dump.py-test
tid=5808 'Simple-log-msg-Args(1,2)' arg1=1 arg2=2
tid=5808 'Simple-log-msg-Args(3,4)' arg1=3 arg2=4
tid=5808 'Potential memory overwrite (addr, size)' arg1=3735927486 arg2=1024

This one is missing (as it's not being looked-for):

"Invalid buffer handle (addr)", 0xbeefabcd, 0

Sample test-program source l3_dump.py-test.c:

#define _POSIX_C_SOURCE 199309L

#include <stdlib.h>
#include <time.h>
#include <inttypes.h>
#include <stdio.h>

#include "l3.h"

int
main(const int argc, const char **argv)
{
    int e = l3_init("/tmp/l3.c-small-unit-test.dat");
    if (e) {
        abort();
    }
    l3_log_simple("Simple-log-msg-Args(1,2)", 1, 2);
    l3_log_simple("Simple-log-msg-Args(3,4)", 3, 4);
    l3_log_simple("Potential memory overwrite (addr, size)", 0xdeadbabe, 1024);
    l3_log_simple("Invalid buffer handle (addr)", 0xbeefabcd, 0);

    return 0;
}

Fixed by PR #13 . This commit addresses this eof-handling bugs:

119ba77 Aditya P. Gurajada 17 hours ago Tue, 02-Apr-2024, 12:49:02 PM (Authored: Mon, 01-Apr-2024, 07:47:26 PM)
 [Py] Fix l3_dump.py to unpack all log-entries found in file.

... bolstered by unit-test added in this commit:

e1a79a1 Aditya P. Gurajada 6 days ago Tue, 02-Apr-2024, 12:49:02 PM (Authored: Wed, 27-Mar-2024, 04:18:36 PM)
 [Test] Makefile rules to build and run unit-tests for l3_dump.py to deal with eof

Closed as fixed.