Segfault by empty implicitly-declared set

[tueda]

This is a bug found in the discussion of #545.

An empty implicitly-declared set results in a segfault.

CF f;
S x;
L F = f(x);
id f(x?{}) = x;
.end

Valgrind output

FORM 5.0.0-beta.1 (May 21 2024, v5.0.0-beta.1-66-g83e3d41)  Run: Wed Jul  3 20:56:27 2024

==11465== Invalid read of size 4
==11465==    at 0x23B2D2: CheckWild (wildcard.c:2279)
==11465==    by 0x15C4A6: MatchFunction (function.c:1223)
==11465==    by 0x15D8B3: ScanFunctions (function.c:1775)
==11465==    by 0x15967E: FindRest (findpat.c:1133)
==11465==    by 0x1ACE06: TestMatch (pattern.c:346)
==11465==    by 0x1E022D: Generator (proces.c:3964)
==11465==    by 0x1E0A74: Generator (proces.c:4216)
==11465==    by 0x1E2102: Processor (proces.c:406)
==11465==    by 0x150E42: DoExecute (execute.c:858)
==11465==    by 0x17C690: ExecModule (module.c:291)
==11465==    by 0x1D6D57: PreProcessor (pre.c:1041)
==11465==    by 0x21466A: main (startup.c:1690)
==11465==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

[jodavies]

One can also trigger this crash with

on names;
CF f;
S x;
set xs: ;
L F = f(x);
id f(x?xs) = x;
.end

In the ?{} case, it does seem to have created an empty implicit set.

The manual says that sets are non-empty: the fix here could be to terminate on creation of an empty set generally? But I don't know if there are somehow some useful ways to use an empty set that people use...

[jodavies]

The least intrusive change I think would be to add after here:
https://github.com/vermaseren/form/blob/61ecfdd9655bae0641e6a3a01ae73edfd1a081b9/sources/wildcard.c#L2230
something like

if ( w == 0 ) {
   MesPrint("Error, set in wildcard specification is empty.");
   Terminate(-1);
}

[tueda]

Actually, a set can be empty. Chapter 21 Diagram generation in the manual has examples with an empty set:

Set empty:;

though it is passed to diagrams_ and not used for wildcarding.

[jodavies]

OK, then indeed the check can't be at set creation. The fix above doesn't work in all cases, however. In the following, the presence of the second id statement lets the code run with no error:

on names;
CF f;
S x,y;
set xs: ;
L F = f(1)+f(2)+f(3);
id f(x?{}) = x;
id f(y?{1,2}) = y;
Print +s;
.end

What I also don't yet understand, is that defining set xs: ; removes the definition {}: from the list of Sets from on names;.

[tueda]

The original issue could be resolved by inserting

if ( w == m ) goto NoMatch;

around
https://github.com/vermaseren/form/blob/3262c64592d69c32196b542a3e3c177b45a9545f/sources/wildcard.c#L2256-L2257

but I'm not sure if we can also skip
https://github.com/vermaseren/form/blob/3262c64592d69c32196b542a3e3c177b45a9545f/sources/wildcard.c#L2232
and the difference between NoMatch and NoMnot.

[jodavies]

Should this be a syntax error or valid syntax that just doesn't match?

[tueda]

I think it is nice if empty implicit sets are syntactically allowed and diagrams_ can be used with implicit sets like diagrams_(phi4,{},{},{},{p1,...,p99},3,0).

[jodavies]

This should still work surely, the code we look at currently is specifically for set restrictions in pattern matching. If one can dynamically generate an empty set (with dollar vars maybe?) then it would be nicer if this was a non-match rather than a syntax error probably.

I think dollar vars are currently not allowed in the set definition, though.

[jodavies]

This example doesn't crash

on names;
CF f;
S x,y;
set xs: ;
L F = f(1)+f(2)+f(3);
id f(x?{}) = x;
id f(y?{1,2}) = y;
Print +s;
.end

because the SetElements macro is not zero any more, so *w is not a null dereference. So checking w==0 is not a good check, and your w==m is better.

Currently in this example, actually, if newnumber is accidentally SetElements (unlikely, it is a memory address) the empty-set id statement will match?