1.9.2: core dump in self tests

[0-wiz-0]

trafficstars

When running the self tests on NetBSD 10.99.3/amd64, I see a segfault:

PASS test_notification_referencing:  (0 ticks, 0.000 sec)
bash: line 2: 14069 Segmentation fault      (core dumped) ./test/test -v
     11348 Done                    | ./test/greenest.awk
gmake: *** [Makefile:76: test] Error 139

[bynect]

could you add the result of test-valgrind please?

[0-wiz-0]

Sorry, no, valgrind does not support NetBSD.

[0-wiz-0]

But here's a backtrace of the core dump:

Core was generated by `test'.
Program terminated with signal SIGSEGV, Segmentation fault.
t#0  0x0000770644f242c2 in _cairo_surface_is_image (surface=<optimized out>) at ../src/cairo-image-surface-inline.h:77
77      ../src/cairo-image-surface-inline.h: No such file or directory.
(gdb) bt
#0  0x0000770644f242c2 in _cairo_surface_is_image (surface=<optimized out>) at ../src/cairo-image-surface-inline.h:77
#1  cairo_image_surface_get_width (surface=0x0) at ../src/cairo-image-surface.c:641
#2  0x0000000000914ad7 in get_icon_width (icon=<optimized out>, scale=scale@entry=1) at test/../src/icon.c:87
#3  0x000000000091dbb0 in test_notification_icon_scaling_toosmall () at test/notification.c:154
#4  suite_notification () at test/notification.c:240
#5  0x00000000009306a0 in greatest_run_suite (suite_name=<optimized out>, suite_cb=0x91d3ba <suite_notification>) at test/test.c:33
#6  greatest_run_suite (suite_cb=0x91d3ba <suite_notification>, suite_name=<optimized out>) at test/test.c:33
#7  0x000000000093789b in main (argc=2, argv=0x7f7fff2ec678) at test/test.c:57

[bynect]

I see. I am not sure but this seems similar to a problem I have encountered (#1228)

[bynect]

I made a temporary fix #1269 which is merged on master. Could you try doing this on the master branch to see if it is resolved?

[0-wiz-0]

git head completes the test suite for me, with:

Pass: 185, fail: 5, skip: 0.

and the following tests failing (but no core dumps):

FAIL test_notification_icon_scaling_toosmall: n->icon (test/notification.c:154) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_toolarge: n->icon (test/notification.c:168) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notconfigured: n->icon (test/notification.c:181) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notneeded: n->icon (test/notification.c:194) (0 ticks, 0.000 sec)
...
FAIL test_pattern_match: rule_field_matches_string("asd", "") (test/rules.c:44) (0 ticks, 0.000 sec)

I had to remove the features.h include from test/utils.c because that header file doesn't exist on my system. Compilation worked fine anyway.

[bynect]

While I don't know why FAIL test_pattern_match: rule_field_matches_string("asd", "") (test/rules.c:44) (0 ticks, 0.000 sec) failed, for the others it was the same problem as me.

basically the icon fails to load and then the test explodes trying to use a null value. Adding a check for null prevents the segfault at least. The reason as to why the icon is not loaded still eludes me, but maybe you don't have the right icon in the project so they are not found?

[0-wiz-0]

Which icon is it trying to load? Which icon set is usually used to provide that?

[0-wiz-0]

I think the code tries to check if the regex "" matches "asd". NetBSD's man page is quite clear that a regular expression needs to have at least one character: https://man.netbsd.org/re_format.7

A (modern) RE is one- or more non-empty- branches, separated by `|'.

I also checked POSIX https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_04 and it's not quite as clear to me but I think it says the same. If you follow the grammar:

basic_reg_exp  :          RE_expression
               | L_ANCHOR
               |                        R_ANCHOR
               | L_ANCHOR               R_ANCHOR
               | L_ANCHOR RE_expression
               |          RE_expression R_ANCHOR
               | L_ANCHOR RE_expression R_ANCHOR
               ;
RE_expression  :               simple_RE
               | RE_expression simple_RE
               ;
simple_RE      : nondupl_RE
               | nondupl_RE RE_dupl_symbol
               ;
nondupl_RE     : one_char_or_coll_elem_RE
               | Back_open_paren RE_expression Back_close_paren
               | BACKREF

the name one_char_or_coll_elem_RE seems to imply that there needs to be something here, but perhaps if you go further down you find out it doesn't.

Anyway, at least on NetBSD "" is not a valid regular expression, so perhaps adapt the test?

[bynect]

I think the code tries to check if the regex "" matches "asd". NetBSD's man page is quite clear that a regular expression needs to have at least one character: https://man.netbsd.org/re_format.7

A (modern) RE is one- or more non-empty- branches, separated by `|'.

I also checked POSIX https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_04 and it's not quite as clear to me but I think it says the same. If you follow the grammar:

basic_reg_exp  :          RE_expression
               | L_ANCHOR
               |                        R_ANCHOR
               | L_ANCHOR               R_ANCHOR
               | L_ANCHOR RE_expression
               |          RE_expression R_ANCHOR
               | L_ANCHOR RE_expression R_ANCHOR
               ;
RE_expression  :               simple_RE
               | RE_expression simple_RE
               ;
simple_RE      : nondupl_RE
               | nondupl_RE RE_dupl_symbol
               ;
nondupl_RE     : one_char_or_coll_elem_RE
               | Back_open_paren RE_expression Back_close_paren
               | BACKREF

the name one_char_or_coll_elem_RE seems to imply that there needs to be something here, but perhaps if you go further down you find out it doesn't.

Anyway, at least on NetBSD "" is not a valid regular expression, so perhaps adapt the test?

You are totally right, I think that tests relies on some undefined behavior on the linux regex implementation. So it should probably be changed altogheter

[bynect]

Which icon is it trying to load? Which icon set is usually used to provide that?

The code that loads the icon is

char *path = g_strconcat(base, "/data/icons/valid.svg", NULL);

where base is

        char *prog = realpath(argv[0], NULL);
//...
        base = dirname(prog);

So it should always refer to the test directory itself. The icon in question is test/data/icons/valid.svg

[0-wiz-0]

With some printf debugging and guessing: The problem is that the file is an SVG. When I changed it to png, two of the tests started passing (the PNG file has different dimensions than the expected SVG file). And when I install the librsvg package that includes the gdk-pixbuf loader for SVG files, all four tests start working. I think the best solution is to just add librsvg or the gdk-pixbuf loader for SVG (I don't know how this is usually packaged in Linux distros) as a requirement in the README.md file.

[bynect]

With some printf debugging and guessing: The problem is that the file is an SVG. When I changed it to png, two of the tests started passing (the PNG file has different dimensions than the expected SVG file). And when I install the librsvg package that includes the gdk-pixbuf loader for SVG files, all four tests start working. I think the best solution is to just add librsvg or the gdk-pixbuf loader for SVG (I don't know how this is usually packaged in Linux distros) as a requirement in the README.md file.

The fact that this is not included in the dependencies is quite strange honestly, I will have to look into that

[bynect]

@fwsmit how can we require librsvg? It is a runtime dependency of gdk-pixbuf. The tests and dunst won't load svg icons if it isn't present

[fwsmit]

Dependencies should be documented in the README. Currently they are not all well documented I think. If it's possible to run without the dependcy you could build a runtime check.

[bynect]

does this still occur?

[0-wiz-0]

With 1.10.0, if librsvg is not installed, yes:

FAIL test_notification_icon_scaling_toosmall: n->icon (test/notification.c:154) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_toolarge: n->icon (test/notification.c:168) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notconfigured: n->icon (test/notification.c:181) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notneeded: n->icon (test/notification.c:194) (0 ticks, 0.000 sec)

[bynect]

With 1.10.0, if librsvg is not installed, yes:

FAIL test_notification_icon_scaling_toosmall: n->icon (test/notification.c:154) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_toolarge: n->icon (test/notification.c:168) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notconfigured: n->icon (test/notification.c:181) (0 ticks, 0.000 sec)
FAIL test_notification_icon_scaling_notneeded: n->icon (test/notification.c:194) (0 ticks, 0.000 sec)

I will change the fail to a skip since it does not seem to be an error on our code 👍🏻

[bynect]

could you try #1329 ? it should handle this situation gracefully

[0-wiz-0]

Manually applying #1329 on top of 1.10.0 makes the tests succeed, even without librsvg installed:

PASS test_notification_icon_scaling_toosmall:  (0 ticks, 0.000 sec)
PASS test_notification_icon_scaling_toolarge:  (0 ticks, 0.000 sec)
PASS test_notification_icon_scaling_notconfigured:  (0 ticks, 0.000 sec)
PASS test_notification_icon_scaling_notneeded:  (0 ticks, 0.000 sec)

Thank you!

[bynect]

Thanks for trying it, I'll merge that pr 👍🏻