Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make easier getting coverage data for devices #63826

Merged
merged 3 commits into from
Oct 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 25 additions & 5 deletions scripts/pylib/twister/twisterlib/coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def retrieve_gcov_data(input_file):

@staticmethod
def create_gcda_files(extracted_coverage_info):
gcda_created = True
logger.debug("Generating gcda files")
for filename, hexdump_val in extracted_coverage_info.items():
# if kobject_hash is given for coverage gcovr fails
Expand All @@ -83,19 +84,33 @@ def create_gcda_files(extracted_coverage_info):
pass
continue

with open(filename, 'wb') as fp:
fp.write(bytes.fromhex(hexdump_val))
try:
with open(filename, 'wb') as fp:
fp.write(bytes.fromhex(hexdump_val))
except ValueError:
logger.exception("Unable to convert hex data for file: {}".format(filename))
gcda_created = False
except FileNotFoundError:
logger.exception("Unable to create gcda file: {}".format(filename))
gcda_created = False
return gcda_created

def generate(self, outdir):
coverage_completed = True
for filename in glob.glob("%s/**/handler.log" % outdir, recursive=True):
gcov_data = self.__class__.retrieve_gcov_data(filename)
capture_complete = gcov_data['complete']
extracted_coverage_info = gcov_data['data']
if capture_complete:
self.__class__.create_gcda_files(extracted_coverage_info)
logger.debug("Gcov data captured: {}".format(filename))
gcda_created = self.__class__.create_gcda_files(extracted_coverage_info)
if gcda_created:
logger.debug("Gcov data captured: {}".format(filename))
else:
logger.error("Gcov data invalid for: {}".format(filename))
coverage_completed = False
else:
logger.error("Gcov data capture incomplete: {}".format(filename))
coverage_completed = False

with open(os.path.join(outdir, "coverage.log"), "a") as coveragelog:
ret = self._generate(outdir, coveragelog)
Expand All @@ -111,6 +126,10 @@ def generate(self, outdir):
}
for r in self.output_formats.split(','):
logger.info(report_log[r])
else:
coverage_completed = False
logger.debug("All coverage data processed: {}".format(coverage_completed))
return coverage_completed


class Lcov(CoverageTool):
Expand Down Expand Up @@ -280,4 +299,5 @@ def run_coverage(testplan, options):
coverage_tool.add_ignore_file('generated')
coverage_tool.add_ignore_directory('tests')
coverage_tool.add_ignore_directory('samples')
coverage_tool.generate(options.outdir)
coverage_completed = coverage_tool.generate(options.outdir)
return coverage_completed
7 changes: 7 additions & 0 deletions scripts/pylib/twister/twisterlib/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,13 @@ def __init__(self, instance, type_str):
"""
super().__init__(instance, type_str)

def get_test_timeout(self):
timeout = super().get_test_timeout()
if self.options.coverage:
# wait more for gcov data to be dumped on console
timeout += 120
return timeout

def monitor_serial(self, ser, halt_event, harness):
log_out_fp = open(self.log, "wb")

Expand Down
4 changes: 3 additions & 1 deletion scripts/pylib/twister/twisterlib/twister_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,10 @@ def main(options):

report.summary(runner.results, options.disable_unrecognized_section_test, duration)

coverage_completed = True
if options.coverage:
if not options.build_only:
run_coverage(tplan, options)
coverage_completed = run_coverage(tplan, options)
else:
logger.info("Skipping coverage report generation due to --build-only.")

Expand All @@ -235,6 +236,7 @@ def main(options):
runner.results.failed
or runner.results.error
or (tplan.warnings and options.warnings_as_errors)
or (options.coverage and not coverage_completed)
):
return 1

Expand Down
12 changes: 11 additions & 1 deletion subsys/testsuite/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,24 @@ config COVERAGE_GCOV_HEAP_SIZE
default 16384
help
This option configures the heap size allocated for gcov coverage
data to be dumped over serial.
data to be dumped over serial. If the value is 0, no buffer will be used,
data will be dumped directly over serial.

config COVERAGE_DUMP
bool "Dump coverage data on exit"
depends on COVERAGE_GCOV
help
Dump collected coverage information to console on exit.

config FORCE_COVERAGE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this if you can just set 'CONFIG_HAS_COVERAGE_SUPPORT'?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the question ;)
That was actually my first attempt, which failed as this config cannot be directly modified:
https://docs.zephyrproject.org/latest/kconfig.html#CONFIG_HAS_COVERAGE_SUPPORT

I decided to add another config that influences HAS_COVERAGE_SUPPORT to describe my intention clearly:
force coverage data generation knowing that it is not supported by design by this platform.
I agree with the current design, that this config is a property of the platform, not an on/off feature.

Do you have suggestions on how this can be implemented differently?
There are already multiple platforms that have this property, I did not want to influence them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, with FORCE_COVERAGE, HAS_COVERAGE_SUPPORT will have no meaning, so, my suggestion is to expand the help message for th is kconfig saying that there is no guarantee coverage data will be produced and that things might fail, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added more information in the help message + rebased.

bool "Force coverage"
select HAS_COVERAGE_SUPPORT
help
Regardless of platform support, it will enable coverage data production.
If the platform does not support coverage by default, setting this config
does not guarantee that coverage data will be gathered.
Application may not fit memory or crash at runtime.

config TEST_USERSPACE
bool "Indicate that this test exercises user mode"
help
Expand Down
149 changes: 89 additions & 60 deletions subsys/testsuite/coverage/coverage.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,54 @@ void __gcov_exit(void)
}

/**
* buff_write_u64 - Store 64 bit data on a buffer and return the size
* print_u8 - Print 8 bit of gcov data
*/
static inline void print_u8(uint8_t v)
{
printk("%02x", v);
}

/**
* print_u32 - Print 32 bit of gcov data
*/
static inline void print_u32(uint32_t v)
{
uint8_t *ptr = (uint8_t *)&v;

print_u8(*ptr);
print_u8(*(ptr+1));
print_u8(*(ptr+2));
print_u8(*(ptr+3));
}

/**
* write_u64 - Store 64 bit data on a buffer and return the size
*/

static inline void buff_write_u64(void *buffer, size_t *off, uint64_t v)
static inline void write_u64(void *buffer, size_t *off, uint64_t v)
{
memcpy((uint8_t *)buffer + *off, (uint8_t *)&v, sizeof(v));
if (buffer != NULL) {
memcpy((uint8_t *)buffer + *off, (uint8_t *)&v, sizeof(v));
} else {
print_u32(*((uint32_t *)&v));
print_u32(*(((uint32_t *)&v) + 1));
}
*off = *off + sizeof(uint64_t);
}

/**
* buff_write_u32 - Store 32 bit data on a buffer and return the size
* write_u32 - Store 32 bit data on a buffer and return the size
*/
static inline void buff_write_u32(void *buffer, size_t *off, uint32_t v)
static inline void write_u32(void *buffer, size_t *off, uint32_t v)
{
memcpy((uint8_t *)buffer + *off, (uint8_t *)&v, sizeof(v));
if (buffer != NULL) {
memcpy((uint8_t *)buffer + *off, (uint8_t *)&v, sizeof(v));
} else {
print_u32(v);
}
*off = *off + sizeof(uint32_t);
}


size_t calculate_buff_size(struct gcov_info *info)
{
uint32_t iter;
Expand Down Expand Up @@ -102,14 +131,13 @@ size_t calculate_buff_size(struct gcov_info *info)
return size;
}


/**
* populate_buffer - convert from gcov data set (info) to
* gcov_to_gcda - convert from gcov data set (info) to
* .gcda file format.
* This buffer will now have info similar to a regular gcda
* format.
*/
size_t populate_buffer(uint8_t *buffer, struct gcov_info *info)
size_t gcov_to_gcda(uint8_t *buffer, struct gcov_info *info)
{
struct gcov_fn_info *functions;
struct gcov_ctr_info *counters_per_func;
Expand All @@ -119,22 +147,22 @@ size_t populate_buffer(uint8_t *buffer, struct gcov_info *info)
size_t buffer_write_position = 0;

/* File header. */
buff_write_u32(buffer,
&buffer_write_position,
GCOV_DATA_MAGIC);
write_u32(buffer,
&buffer_write_position,
GCOV_DATA_MAGIC);

buff_write_u32(buffer,
&buffer_write_position,
info->version);
write_u32(buffer,
&buffer_write_position,
info->version);

buff_write_u32(buffer,
&buffer_write_position,
info->stamp);
write_u32(buffer,
&buffer_write_position,
info->stamp);

#ifdef GCOV_12_FORMAT
buff_write_u32(buffer,
&buffer_write_position,
info->checksum);
write_u32(buffer,
&buffer_write_position,
info->checksum);
#endif

for (iter_functions = 0U;
Expand All @@ -144,25 +172,25 @@ size_t populate_buffer(uint8_t *buffer, struct gcov_info *info)
functions = info->functions[iter_functions];


buff_write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FUNCTION);
write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FUNCTION);

buff_write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FUNCTION_LENGTH);
write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FUNCTION_LENGTH);

buff_write_u32(buffer,
&buffer_write_position,
functions->ident);
write_u32(buffer,
&buffer_write_position,
functions->ident);

buff_write_u32(buffer,
&buffer_write_position,
functions->lineno_checksum);
write_u32(buffer,
&buffer_write_position,
functions->lineno_checksum);

buff_write_u32(buffer,
&buffer_write_position,
functions->cfg_checksum);
write_u32(buffer,
&buffer_write_position,
functions->cfg_checksum);

counters_per_func = functions->ctrs;

Expand All @@ -174,29 +202,28 @@ size_t populate_buffer(uint8_t *buffer, struct gcov_info *info)
continue;
}

buff_write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FOR_COUNTER(iter_counts));
write_u32(buffer,
&buffer_write_position,
GCOV_TAG_FOR_COUNTER(iter_counts));

#ifdef GCOV_12_FORMAT
/* GCOV 12 counts the length by bytes */
buff_write_u32(buffer,
&buffer_write_position,
counters_per_func->num * 2U * 4);
write_u32(buffer,
&buffer_write_position,
counters_per_func->num * 2U * 4);
#else
buff_write_u32(buffer,
&buffer_write_position,
counters_per_func->num * 2U);
write_u32(buffer,
&buffer_write_position,
counters_per_func->num * 2U);
#endif

for (iter_counter_values = 0U;
iter_counter_values < counters_per_func->num;
iter_counter_values++) {

buff_write_u64(buffer,
&buffer_write_position,
counters_per_func->\
values[iter_counter_values]);
write_u64(buffer,
&buffer_write_position,
counters_per_func->values[iter_counter_values]);
}

counters_per_func++;
Expand All @@ -205,20 +232,21 @@ size_t populate_buffer(uint8_t *buffer, struct gcov_info *info)
return buffer_write_position;
}

void dump_on_console(const char *filename, char *ptr, size_t len)
void dump_on_console_start(const char *filename)
{
uint32_t iter;

printk("\n%c", FILE_START_INDICATOR);
while (*filename != '\0') {
printk("%c", *filename++);
}
printk("%c", GCOV_DUMP_SEPARATOR);
}

/* Data dump */

for (iter = 0U; iter < len; iter++) {
printk(" %02x", (uint8_t)*ptr++);
void dump_on_console_data(char *ptr, size_t len)
{
if (ptr != NULL) {
for (size_t iter = 0U; iter < len; iter++) {
print_u8((uint8_t)*ptr++);
}
}
}

Expand All @@ -237,21 +265,22 @@ void gcov_coverage_dump(void)
printk("\nGCOV_COVERAGE_DUMP_START");
while (gcov_list) {

dump_on_console_start(gcov_list->filename);
golowanow marked this conversation as resolved.
Show resolved Hide resolved
size = calculate_buff_size(gcov_list);

buffer = k_heap_alloc(&gcov_heap, size, K_NO_WAIT);
if (!buffer) {
if (CONFIG_COVERAGE_GCOV_HEAP_SIZE > 0 && !buffer) {
printk("No Mem available to continue dump\n");
goto coverage_dump_end;
}

written_size = populate_buffer(buffer, gcov_list);
written_size = gcov_to_gcda(buffer, gcov_list);
if (written_size != size) {
printk("Write Error on buff\n");
goto coverage_dump_end;
}

dump_on_console(gcov_list->filename, buffer, size);
dump_on_console_data(buffer, size);

k_heap_free(&gcov_heap, buffer);
gcov_list = gcov_list->next;
Expand Down
Loading