[bpf-next] selftests/bpf: fix trampoline_count clean up logic
diff mbox series

Message ID 20200220230546.769250-1-andriin@fb.com
State Accepted
Delegated to: BPF Maintainers
Headers show
Series
  • [bpf-next] selftests/bpf: fix trampoline_count clean up logic
Related show

Commit Message

Andrii Nakryiko Feb. 20, 2020, 11:05 p.m. UTC
Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
clean up is performed correctly.

Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
Cc: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
 1 file changed, 18 insertions(+), 7 deletions(-)

Comments

Alexei Starovoitov Feb. 21, 2020, 2:04 a.m. UTC | #1
On Thu, Feb 20, 2020 at 03:05:46PM -0800, Andrii Nakryiko wrote:
> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> clean up is performed correctly.
> 
> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Applied, Thanks
Song Liu Feb. 21, 2020, 2:06 a.m. UTC | #2
On Thu, Feb 20, 2020 at 3:07 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> clean up is performed correctly.
>
> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 1f6ccdaed1ac..781c8d11604b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>         /* attach 'allowed' 40 trampoline programs */
>         for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>                 obj = bpf_object__open_file(object, NULL);
> -               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +                       obj = NULL;

I think we don't need obj and link in cleanup? Did I miss anything?
Andrii Nakryiko Feb. 21, 2020, 4:20 a.m. UTC | #3
On 2/20/20 6:06 PM, Song Liu wrote:
> On Thu, Feb 20, 2020 at 3:07 PM Andrii Nakryiko <andriin@fb.com> wrote:
>>
>> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
>> clean up is performed correctly.
>>
>> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
>> Cc: Jiri Olsa <jolsa@kernel.org>
>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>> ---
>>   .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>>   1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>> index 1f6ccdaed1ac..781c8d11604b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>>          /* attach 'allowed' 40 trampoline programs */
>>          for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>>                  obj = bpf_object__open_file(object, NULL);
>> -               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
>> +               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
>> +                       obj = NULL;
> 
> I think we don't need obj and link in cleanup? Did I miss anything?
> 

We do set obj below (line 87) after this loop, so need to clean it up as 
well. As for link, yeah, technically link doesn't have to be set to 
NULL, but I kind of did it for completeness without thinking too much.
Song Liu Feb. 21, 2020, 4:31 a.m. UTC | #4
> On Feb 20, 2020, at 8:20 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> 
> On 2/20/20 6:06 PM, Song Liu wrote:
>> On Thu, Feb 20, 2020 at 3:07 PM Andrii Nakryiko <andriin@fb.com> wrote:
>>> 
>>> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
>>> clean up is performed correctly.
>>> 
>>> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> ---
>>>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>> 
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>>> index 1f6ccdaed1ac..781c8d11604b 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
>>> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>>>         /* attach 'allowed' 40 trampoline programs */
>>>         for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>>>                 obj = bpf_object__open_file(object, NULL);
>>> -               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
>>> +               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
>>> +                       obj = NULL;
>> I think we don't need obj and link in cleanup? Did I miss anything?
> 
> We do set obj below (line 87) after this loop, so need to clean it up as well. As for link, yeah, technically link doesn't have to be set to NULL, but I kind of did it for completeness without thinking too much.

I meant "obj = NULL;" before "goto cleanup;", as we don't use obj in the 
cleanup path. 

Anyway, this is not a real issue. 

Thanks,
Song
Andrii Nakryiko Feb. 21, 2020, 4:45 a.m. UTC | #5
On Thu, Feb 20, 2020 at 8:31 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Feb 20, 2020, at 8:20 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > On 2/20/20 6:06 PM, Song Liu wrote:
> >> On Thu, Feb 20, 2020 at 3:07 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >>>
> >>> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> >>> clean up is performed correctly.
> >>>
> >>> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> >>> Cc: Jiri Olsa <jolsa@kernel.org>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >>> ---
> >>>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
> >>>  1 file changed, 18 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> >>> index 1f6ccdaed1ac..781c8d11604b 100644
> >>> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> >>> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> >>> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
> >>>         /* attach 'allowed' 40 trampoline programs */
> >>>         for (i = 0; i < MAX_TRAMP_PROGS; i++) {
> >>>                 obj = bpf_object__open_file(object, NULL);
> >>> -               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> >>> +               if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> >>> +                       obj = NULL;
> >> I think we don't need obj and link in cleanup? Did I miss anything?
> >
> > We do set obj below (line 87) after this loop, so need to clean it up as well. As for link, yeah, technically link doesn't have to be set to NULL, but I kind of did it for completeness without thinking too much.
>
> I meant "obj = NULL;" before "goto cleanup;", as we don't use obj in the
> cleanup path.
>
> Anyway, this is not a real issue.

Ah, I see what you are saying, we skip over that bpf_object__close()
call, right.

>
> Thanks,
> Song
>
Jakub Sitnicki Feb. 21, 2020, 10:21 a.m. UTC | #6
On Thu, Feb 20, 2020 at 11:05 PM GMT, Andrii Nakryiko wrote:
> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> clean up is performed correctly.
>
> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 1f6ccdaed1ac..781c8d11604b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>  	/* attach 'allowed' 40 trampoline programs */
>  	for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>  		obj = bpf_object__open_file(object, NULL);
> -		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +			obj = NULL;
>  			goto cleanup;
> +		}
>
>  		err = bpf_object__load(obj);
>  		if (CHECK(err, "obj_load", "err %d\n", err))
>  			goto cleanup;
>  		inst[i].obj = obj;
> +		obj = NULL;
>
>  		if (rand() % 2) {
> -			link = load(obj, fentry_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fentry_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fentry = link;
>  		} else {
> -			link = load(obj, fexit_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fexit_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fexit = link;
>  		}
>  	}
>
>  	/* and try 1 extra.. */
>  	obj = bpf_object__open_file(object, NULL);
> -	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +		obj = NULL;
>  		goto cleanup;
> +	}
>
>  	err = bpf_object__load(obj);
>  	if (CHECK(err, "obj_load", "err %d\n", err))
> @@ -104,7 +113,9 @@ void test_trampoline_count(void)
>  cleanup_extra:
>  	bpf_object__close(obj);
>  cleanup:
> -	while (--i) {
> +	if (i >= MAX_TRAMP_PROGS)
> +		i = MAX_TRAMP_PROGS - 1;
> +	for (; i >= 0; i--) {
>  		bpf_link__destroy(inst[i].link_fentry);
>  		bpf_link__destroy(inst[i].link_fexit);
>  		bpf_object__close(inst[i].obj);

I'm not sure I'm grasping what the fix is about.

We don't access obj or link from cleanup, so what is the point of
setting them to NULL before jumping there?

Or is it all just an ancillary change to clamping the loop index
variable to (MAX_TRAMP_PROGS - 1)?
Jiri Olsa Feb. 21, 2020, 1:04 p.m. UTC | #7
On Thu, Feb 20, 2020 at 03:05:46PM -0800, Andrii Nakryiko wrote:
> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> clean up is performed correctly.
> 
> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> Cc: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 1f6ccdaed1ac..781c8d11604b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>  	/* attach 'allowed' 40 trampoline programs */
>  	for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>  		obj = bpf_object__open_file(object, NULL);
> -		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +			obj = NULL;
>  			goto cleanup;
> +		}
>  
>  		err = bpf_object__load(obj);
>  		if (CHECK(err, "obj_load", "err %d\n", err))
>  			goto cleanup;
>  		inst[i].obj = obj;
> +		obj = NULL;
>  
>  		if (rand() % 2) {
> -			link = load(obj, fentry_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fentry_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fentry = link;
>  		} else {
> -			link = load(obj, fexit_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fexit_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fexit = link;
>  		}
>  	}
>  
>  	/* and try 1 extra.. */
>  	obj = bpf_object__open_file(object, NULL);
> -	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +		obj = NULL;
>  		goto cleanup;
> +	}
>  
>  	err = bpf_object__load(obj);
>  	if (CHECK(err, "obj_load", "err %d\n", err))
> @@ -104,7 +113,9 @@ void test_trampoline_count(void)
>  cleanup_extra:
>  	bpf_object__close(obj);
>  cleanup:
> -	while (--i) {
> +	if (i >= MAX_TRAMP_PROGS)
> +		i = MAX_TRAMP_PROGS - 1;
> +	for (; i >= 0; i--) {

ugh right, if we fail in first iteration, 'i' would get get -1 in here

thanks,
jirka

>  		bpf_link__destroy(inst[i].link_fentry);
>  		bpf_link__destroy(inst[i].link_fexit);
>  		bpf_object__close(inst[i].obj);
> -- 
> 2.17.1
>
Andrii Nakryiko Feb. 22, 2020, 12:21 a.m. UTC | #8
On Fri, Feb 21, 2020 at 2:21 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:
>
> On Thu, Feb 20, 2020 at 11:05 PM GMT, Andrii Nakryiko wrote:
> > Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> > clean up is performed correctly.
> >
> > Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
> >  1 file changed, 18 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > index 1f6ccdaed1ac..781c8d11604b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> > @@ -55,31 +55,40 @@ void test_trampoline_count(void)
> >       /* attach 'allowed' 40 trampoline programs */
> >       for (i = 0; i < MAX_TRAMP_PROGS; i++) {
> >               obj = bpf_object__open_file(object, NULL);
> > -             if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> > +             if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> > +                     obj = NULL;
> >                       goto cleanup;
> > +             }
> >
> >               err = bpf_object__load(obj);
> >               if (CHECK(err, "obj_load", "err %d\n", err))
> >                       goto cleanup;
> >               inst[i].obj = obj;
> > +             obj = NULL;
> >
> >               if (rand() % 2) {
> > -                     link = load(obj, fentry_name);
> > -                     if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> > +                     link = load(inst[i].obj, fentry_name);
> > +                     if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> > +                             link = NULL;
> >                               goto cleanup;
> > +                     }
> >                       inst[i].link_fentry = link;
> >               } else {
> > -                     link = load(obj, fexit_name);
> > -                     if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> > +                     link = load(inst[i].obj, fexit_name);
> > +                     if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> > +                             link = NULL;
> >                               goto cleanup;
> > +                     }
> >                       inst[i].link_fexit = link;
> >               }
> >       }
> >
> >       /* and try 1 extra.. */
> >       obj = bpf_object__open_file(object, NULL);
> > -     if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> > +     if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> > +             obj = NULL;
> >               goto cleanup;
> > +     }
> >
> >       err = bpf_object__load(obj);
> >       if (CHECK(err, "obj_load", "err %d\n", err))
> > @@ -104,7 +113,9 @@ void test_trampoline_count(void)
> >  cleanup_extra:
> >       bpf_object__close(obj);
> >  cleanup:
> > -     while (--i) {
> > +     if (i >= MAX_TRAMP_PROGS)
> > +             i = MAX_TRAMP_PROGS - 1;
> > +     for (; i >= 0; i--) {
> >               bpf_link__destroy(inst[i].link_fentry);
> >               bpf_link__destroy(inst[i].link_fexit);
> >               bpf_object__close(inst[i].obj);
>
> I'm not sure I'm grasping what the fix is about.
>
> We don't access obj or link from cleanup, so what is the point of
> setting them to NULL before jumping there?
>
> Or is it all just an ancillary change to clamping the loop index
> variable to (MAX_TRAMP_PROGS - 1)?

Yeah, mostly ancillary. But this is not just clamping to
MAX_TRAMP_PROGS-1. As Jiri pointed out, it's also handling a case of i
== 0.

Patch
diff mbox series

diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
index 1f6ccdaed1ac..781c8d11604b 100644
--- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
+++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
@@ -55,31 +55,40 @@  void test_trampoline_count(void)
 	/* attach 'allowed' 40 trampoline programs */
 	for (i = 0; i < MAX_TRAMP_PROGS; i++) {
 		obj = bpf_object__open_file(object, NULL);
-		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
+		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
+			obj = NULL;
 			goto cleanup;
+		}
 
 		err = bpf_object__load(obj);
 		if (CHECK(err, "obj_load", "err %d\n", err))
 			goto cleanup;
 		inst[i].obj = obj;
+		obj = NULL;
 
 		if (rand() % 2) {
-			link = load(obj, fentry_name);
-			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
+			link = load(inst[i].obj, fentry_name);
+			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
+				link = NULL;
 				goto cleanup;
+			}
 			inst[i].link_fentry = link;
 		} else {
-			link = load(obj, fexit_name);
-			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
+			link = load(inst[i].obj, fexit_name);
+			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
+				link = NULL;
 				goto cleanup;
+			}
 			inst[i].link_fexit = link;
 		}
 	}
 
 	/* and try 1 extra.. */
 	obj = bpf_object__open_file(object, NULL);
-	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
+	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
+		obj = NULL;
 		goto cleanup;
+	}
 
 	err = bpf_object__load(obj);
 	if (CHECK(err, "obj_load", "err %d\n", err))
@@ -104,7 +113,9 @@  void test_trampoline_count(void)
 cleanup_extra:
 	bpf_object__close(obj);
 cleanup:
-	while (--i) {
+	if (i >= MAX_TRAMP_PROGS)
+		i = MAX_TRAMP_PROGS - 1;
+	for (; i >= 0; i--) {
 		bpf_link__destroy(inst[i].link_fentry);
 		bpf_link__destroy(inst[i].link_fexit);
 		bpf_object__close(inst[i].obj);