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

TTQ-788 Dev fuzzer malloc #429

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ ifeq ($(ARCH),faulty)
DUMMY_EXTRA+=test/pico_faulty.o
endif

ifdef CHECK_MEM
CFLAGS+=-DCHECK_MEM -rdynamic
endif

ifeq ($(ARCH),msp430)
CFLAGS+=-DMSP430
endif
Expand Down
46 changes: 44 additions & 2 deletions include/pico_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@
#ifndef __KERNEL__
#include <stdint.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#else
#include <linux/types.h>
#endif
#ifdef __linux__
#include <execinfo.h>
#endif


#if defined __IAR_SYSTEMS_ICC__ || defined ATOP
# define PACKED_STRUCT_DEF __packed struct
Expand Down Expand Up @@ -51,6 +56,8 @@
#define long_be(x) (x)
#define long_long_be(x) (x)



static inline uint16_t short_from(void *_p)
{
unsigned char *p = (unsigned char *)_p;
Expand Down Expand Up @@ -163,7 +170,6 @@ static inline uint64_t long_long_be(uint64_t le)
# endif /* BYTESWAP_GCC */
#endif


/* Mockables */
#if defined UNIT_TEST
# define MOCKABLE __attribute__((weak))
Expand Down Expand Up @@ -229,7 +235,43 @@ static inline uint64_t long_long_be(uint64_t le)
# include "arch/pico_posix.h"
#endif

#ifdef PICO_SUPPORT_MM

#ifdef CHECK_MEM
extern int start_failing_mallocs;

static inline void log_malloc(const char* filename, const char* message)
{
FILE * file;
file = fopen(filename, "a");
fprintf(file, message);
fclose(file);
}

static inline void append_backtrace(const char* filename)
{
void *array[10];
size_t size;
int fd;
FILE * file;
file = fopen(filename, "a");
fd = fileno(file);

fprintf(file, "Backtrace:\n");
fseek(file, 0, SEEK_END);

size = backtrace(array, 10);
backtrace_symbols_fd(array, size, fd);
fprintf(file, "\n");

fclose(file);
}

#define PICO_ZALLOC(x) \
((start_failing_mallocs && (((double)(rand())/(double)RAND_MAX) < 0.4)) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get the probability of failure out of an environment variable as well? Might be interesting to automatically test with that.

? (log_malloc("mem_test.log", "Malloc FAILED\n"), append_backtrace("mem_test.log"), NULL) \
: (log_malloc("mem_test.log", "Malloc Succeeded\n"), append_backtrace("mem_test.log"), pico_zalloc(x)))
Copy link
Contributor

Choose a reason for hiding this comment

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

An afterthought : it looks like this statement expression is GNU C [0], not entirely standard. In the interest of compiler compatibility, it may be better to just put this into a function...

[0] https://gcc.gnu.org/onlinedocs/gcc-3.3.1/gcc/Statement-Exprs.html

Copy link
Author

Choose a reason for hiding this comment

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

How would you do that? The problem with that is that 'pico_zalloc()' isn't defined yet

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't pico_zalloc() defined in the arch-file that was included a little above this?

Copy link
Author

Choose a reason for hiding this comment

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

Oops you're right. I tried it again. The problem is actually multiple definitions of the function I create since pico_config is compiled into everything. Trying to fix it

Copy link
Contributor

Choose a reason for hiding this comment

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

would inline solve this?

Copy link
Author

Choose a reason for hiding this comment

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

Yup! static inline does. Thanks :)

#define PICO_FREE(x) pico_free(x)
#elif defined PICO_SUPPORT_MM
#define PICO_ZALLOC(x) pico_mem_zalloc(x)
#define PICO_FREE(x) pico_mem_free(x)
#else
Expand Down
17 changes: 16 additions & 1 deletion stack/pico_stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
# define MOCKABLE
#endif


volatile pico_time pico_tick;
volatile pico_err_t pico_err;

Expand Down Expand Up @@ -857,6 +856,20 @@ uint32_t pico_timer_add_hashed(pico_time expire, void (*timer)(pico_time, void *

int MOCKABLE pico_stack_init(void)
{

#ifdef CHECK_MEM
//New log for malloc information
fopen("mem_test.log", "w");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you doing anything with the file you're opening here?

Copy link
Author

Choose a reason for hiding this comment

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

The idea was to make sure to clear the file before running a new set of tests but this was flawed since it was getting called with each new test and so only the last test's mallocs were getting logged in the end. I changed to outputting this in stdout, which also makes it clear which tests the mallocs occurred in and which ASAN errors are related to them.


//Set rand seed
char *buf = getenv("MEM_TEST_SEED");
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you also putting this seed into some kind of log, so that we can try to recreate afterwards?

Copy link
Author

Choose a reason for hiding this comment

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

I'll try to get this done. Just noticed there's a problem of getting the environment variable when I run it through the RF test rather than just the unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could be good to make a single function that reads all environment variables into local/global variables. Then you can also log these immediatly.
What's wrong with getting them in RF tests? it's possible that these set up a virtual environment...

if(buf!=NULL){
srand(atoi(buf));
}
else
srand(1);
#endif

#ifdef PICO_SUPPORT_ETH
pico_protocol_init(&pico_proto_ethernet);
#endif
Expand Down Expand Up @@ -917,9 +930,11 @@ int MOCKABLE pico_stack_init(void)
pico_aodv_init();
#endif


pico_stack_tick();
pico_stack_tick();
pico_stack_tick();

return 0;
}