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

Backtrace improvements for utilities #4116

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

asadsa92
Copy link
Contributor

@asadsa92 asadsa92 commented Jun 5, 2024

This PR adds a default back-trace to all asserts outside of varnishd, e.g., varnishstat.
Also add back-trace to varnishtest asserts.

Example (artificially crafted asserts):


Before:

Assert error in do_once_cb_first(), varnishstat.c line 164:
  Condition((0) != 0) not true.
Aborted

After:

Assert error in do_once_cb_first(), varnishstat.c line 164:
  Condition((0) != 0) not true.
Backtrace:
  ip=0x7f3458bb6125 sp=0x7fff606013e0 <VBT_format+0x15>
  ip=0x7f3458bb66eb sp=0x7fff60601400 <VBT_dump+0x6b>
  ip=0x7f3458bb5394 sp=0x7fff60601450 <vas_default+0x1b4>
  ip=0x7f3458bb51d3 sp=0x7fff606024c0 <VAS_Fail+0x63>
  ip=0x5614ef51548a sp=0x7fff60602500 <do_once_cb_first+0x3a>
  ip=0x7f3458b963d3 sp=0x7fff60602540 <vsc_iter_seg+0x1c3>
  ip=0x7f3458b957bb sp=0x7fff60602590 <VSC_Iter+0x34b>
  ip=0x5614ef51514e sp=0x7fff60602610 <do_once+0xde>
  ip=0x5614ef514a8d sp=0x7fff60602650 <main+0x5ed>
  ip=0x7f345887124a sp=0x7fff606026c0 <__libc_init_first+0x8a>
  ip=0x7f3458871305 sp=0x7fff60602760 <__libc_start_main+0x85>
  ip=0x5614ef5143d1 sp=0x7fff606027b0 <_start+0x21>
Aborted

Before:

*    top   RESETTING after bin/varnishtest/tests/p00000.vtc
**** dT    0.472
**   s1    Waiting for server (5/-1)
**   v1    Wait
**** dT    0.474
---- v1    Assert error in varnish_ask_cli(), vtc_varnish.c line 110:  Condition((0) != 0) not true.  Errno=0 Success
*    top   failure during reset
#    top  TEST bin/varnishtest/tests/p00000.vtc FAILED (0.474) exit=2

After

*    top   RESETTING after bin/varnishtest/tests/p00000.vtc
**** dT    0.472
**   s1    Waiting for server (5/-1)
**   v1    Wait
**** dT    0.474
---- v1    Assert error in varnish_ask_cli(), vtc_varnish.c line 110:  Condition((0) != 0) not true.  Errno=0 Success
Backtrace:
  ip=0x55f0c01f3845 sp=0x7ffd49fc2480 <VBT_format+0x15>
  ip=0x55f0c01f3e0b sp=0x7ffd49fc24a0 <VBT_dump+0x6b>
  ip=0x55f0c01d453f sp=0x7ffd49fc24f0 <vtc_log_VAS_Fail+0x4f>
  ip=0x7fec618c41bb sp=0x7ffd49fc35c0 <VAS_Fail+0x4b>
  ip=0x55f0c01f043e sp=0x7ffd49fc3600 <varnish_ask_cli+0x3e>
  ip=0x55f0c01ee07d sp=0x7ffd49fc3640 <varnish_wait+0x7d>
  ip=0x55f0c01ed132 sp=0x7ffd49fc3660 <cmd_varnish+0x72>
  ip=0x55f0c01b0ddb sp=0x7ffd49fc36d0 <reset_cmds+0x2b>
  ip=0x55f0c01b0d13 sp=0x7ffd49fc36f0 <fail_out+0xa3>
  ip=0x55f0c01d38df sp=0x7ffd49fc3710 <vtc_logfail+0x3f>
  ip=0x55f0c01d3548 sp=0x7ffd49fc3720 <vtc_fatal+0x278>
  ip=0x55f0c01d4707 sp=0x7ffd49fc3830 <vtc_log_VAS_Fail+0x217>
  ip=0x7fec618c41bb sp=0x7ffd49fc4900 <VAS_Fail+0x4b>
  ip=0x55f0c01f043e sp=0x7ffd49fc4940 <varnish_ask_cli+0x3e>
  ip=0x55f0c01f1460 sp=0x7ffd49fc4980 <varnish_launch+0x1020>
  ip=0x55f0c01efcaa sp=0x7ffd49fc5aa0 <varnish_vclbackend+0xaa>
  ip=0x55f0c01ede7f sp=0x7ffd49fc5b00 <cmd_varnish+0xdbf>
  ip=0x55f0c01b0c43 sp=0x7ffd49fc5b70 <parse_string+0xc43>
  ip=0x55f0c01b12c3 sp=0x7ffd49fc6870 <exec_file+0x4c3>
  ip=0x55f0c01dbc01 sp=0x7ffd49fc68d0 <start_test+0x671>
  ip=0x55f0c01d9ebb sp=0x7ffd49fc7940 <main+0x70b>
  ip=0x7fec6151a24a sp=0x7ffd49fc89b0 <__libc_init_first+0x8a>
  ip=0x7fec6151a305 sp=0x7ffd49fc8a50 <__libc_start_main+0x85>
  ip=0x55f0c01a8eb1 sp=0x7ffd49fc8aa0 <_start+0x21>


*    top   failure during reset
#    top  TEST bin/varnishtest/tests/p00000.vtc FAILED (0.474) exit=2

This is useful when printing a backtrace into a fixed size buffer.

Suggested-by: Dridi Boukelmoune <[email protected]>
Signed-off-by: Asad Sajjad Ahmed <[email protected]>
Signed-off-by: Asad Sajjad Ahmed <[email protected]>
Signed-off-by: Asad Sajjad Ahmed <[email protected]>
Copy link
Member

@dridi dridi left a comment

Choose a reason for hiding this comment

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

The second patch is easier to review with the --ignore-all-space option (or ?w=1 query string on github).

bin/varnishtest/vtc_log.c Outdated Show resolved Hide resolved
Signed-off-by: Asad Sajjad Ahmed <[email protected]>
@dridi
Copy link
Member

dridi commented Jun 10, 2024

bugwash: we should enable back-traces outside of varnishd with some kind of developer mode set in varnishd and exposed to VSM readers with a new Arg segment or something similar.

The developer mode should always be assumed by varnishtest (which is much more than a VSM reader) and as an intermediate measure this could be enabled at configure time and always enabled by autogen.des.

@asadsa92
Copy link
Contributor Author

asadsa92 commented Nov 6, 2024

Bumping this. I assume this can be merged after a rebase?
@dridi I also assume you meant "developer mode" in varnishd can be done later?

@dridi
Copy link
Member

dridi commented Nov 6, 2024

I think @bsdphk insisted that we do not make this the default, that it should be controlled in varnishd but we could fall back to a configure-time option (eg. --enable-vut-bt) until we introduce this developer mode.

So I don't think it was acceptable as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants