aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
...
* Rough support for fuzzing with libFuzzerBen Harris2023-02-23
| | | | | | | | | | | | | | | | For AFL++ and Honggfuzz, our approach is to build a standard fuzzpuzz binary with extra hooks for interacting with an external fuzzer. This works well for AFL++ and tolerably for Honggfuzz. LibFuzzer, though, provides its own main() so that the resulting program has a very different command-line interface from the normal one. Also, since libFuzzer is a standard part of Clang, we can't decide whether to use it based on the behaviour of the compiler. So what I've done, at least for now, is to have CMake detect when we're using Clang and in that case build a separate binary called "fuzzpuzz-libfuzzer" which is built with -fsanitize=fuzzer, while the ordinary fuzzpuzz is built without. I'm not sure if this is the right approach, though.
* Revert "JS puzzles: use the PointerEvent API if available."Simon Tatham2023-02-23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 9d7c2b8c83506c1f239c840e372058fac603b255. I thought that switching from the JS 'mousedown', 'mousemove' and 'mouseup' events to the corresponding 'pointer*' events would make essentially no difference except that the pointer events would come with more information. But in fact it turns out that there's a fundamental change of semantics. If you press one mouse button down and then, without releasing it, press a second one, then the mouse API will send you this information in the form of two 'mousedown' events, one for each button. But the pointer API will only send you a 'pointerdown' for the first event, when the state of the pointer changes from 'no buttons down' to 'at least one button down'. The second button press will be delivered as a 'pointermove', in which the 'buttons' field is different from its previous value. I'm backing out the migration to PointerEvent for the moment, because that's too complicated for a trivial fix. In simple cases we could easily detect the changed buttons field in the pointermove handler and generate a call to the C side of this front end's mousedown() function, effectively converting the changed JS representation to the one the C was already expecting. But this also has to interact with our one-button support (converting Ctrl and Shift clicks into a different logical button) _and_ with the ad-hoc mechanism we use to avoid delivering buttonless mouse movements to the C side. So getting it right in all cases at once isn't trivial, and I'd rather revert the attempt now and think about it later than commit to getting it all perfect on short notice.
* Normalise pathnames in assert statements where possible.Simon Tatham2023-02-22
| | | | | | | | | | | | | | | | After commit 1470c9530b1cff3 enabled assertions, I found that my build scripts were complaining that the Windows binaries built from the same source twice were not generating the same output, and it turned out to be because the use of __FILE__ in every assert was baking in a pathname from my build setup containing a mkstemp()-randomised path component. I've found the '-fmacro-prefix-map' option, available in both gcc and clang (except the archaic gcc used by my NestedVM build, alas), which lets you remap pathnames for purpose of what __FILE__ expands to. So now our assertion statements should look as if the puzzle source just lived in /puzzles, and (just in case a pathname in a generated header ever becomes relevant) the cmake build directory is /build.
* JS puzzles: use the PointerEvent API if available.Simon Tatham2023-02-22
| | | | | | | | | | | | | | | | | If the browser knows what 'PointerEvent' means, then we switch our 'onmousefoo' event handlers to the 'onpointerfoo' events, for both the puzzle canvas and the resize handle. The immediate effect of this is that we get to use the setPointerCapture method on the puzzle canvas, in preference to the deprecated Firefox-only setCapture. A pointer event also contains extra fields compared to a mouse event: as well as telling you which pointing device the event comes from, it can also provide extra information, such as pressure, or the angle of a stylus if the hardware can detect it. I don't have any immediate ideas about what those could be used for, but it can't hurt to have them available just in case we think of something in future.
* Fix error about setCapture not existing.Simon Tatham2023-02-20
| | | | | | | | | | | | | | | | | | | element.setCapture only seems to exist in Firefox. On most other browsers, our attempt to call it must have been generating a whinge in the console log all along. But Ben's commit bb16b5a70ddf77d turned that into a prominent alert box, triggered on every mouse click in the puzzle canvas. Worked around by wrapping both calls to setCapture in a local subroutine which checks if it's there before calling it. Also, setCapture turns out to be deprecated in any case, according to https://developer.mozilla.org/en-US/docs/Web/API/Element/setCapture . It looks as if the non-deprecated version is element.setPointerCapture: https://developer.mozilla.org/en-US/docs/Web/API/Element/setPointerCapture But it also looks as if that needs the 'pointerId' field that's only found in 'onpointerdown' events and not 'onmousedown' ones. So including that as an alternative will be a bigger job.
* Flood: don't read off the end of some parameter stringsBen Harris2023-02-20
| | | | | | | This is essentially the same fix as 73c7bc090155ab8c was for Twiddle. The new code is less clever but more correct (and more obviously correct). The bug could be demonstrated by using a parameter string of "c" or "m" with an AddressSanitizer build of Flood.
* GTK: Free error message if new_window failsBen Harris2023-02-20
| | | | | This is kind of pointless because it comes just before a return from main(), but it's pretty harmless and it cheers up AddressSanitizer.
* Fix memory leak in midend_game_id_int()Ben Harris2023-02-20
| | | | | | The "par" string wasn't getting freed on some error paths. Fixed by freeing it immediately after its last use, which is before any of the error paths.
* Make the HAVE_HF_ITER define target-specificBen Harris2023-02-20
| | | | | Leaking HAVE_HF_ITER into the entire build just because fuzzpuzz wanted it was ugly, and also this needs fewer lines of CMake code.
* Support multiple COMPILE_DEFINITIONS for a programBen Harris2023-02-20
| | | | | | | | Despite the name, COMPILE_DEFINITIONS was only ever used to set a single definition, and as far as I can tell that's all it could do even when I tried to put them in a single word separated by semicolons. Turning COMPILE_DEFINITIONS into a multi-valued argument seems to make it work much better.
* Try to stop CMake disabling assertions in release buildsBen Harris2023-02-19
| | | | | | | | | | Assertion failures are ugly, but they're better than the alternative. Defensive coding is a general principle throughout Puzzles and I don't think it's sensible to selectively turn that off. The mechanism by which we re-enable assertions is stolen from PuTTY (with an enhancement to cover MinSizeRel builds as well) and is pretty ugly because CMake doesn't seem to have a good way to do it.
* js: Add a trivial error handler that alert()sBen Harris2023-02-19
| | | | | | | I'm not quite sure how useful it will be, but it does at least catch an assertion failure in main() and present an opaque message in a box, which is better than stopping and putting a message in the console where no-one will see it.
* Convert a lot of floating-point constants to single precisionBen Harris2023-02-19
| | | | | | | | | | | | | | | | | | For reasons now lost to history, Puzzles generally uses single-precision floating point. However, C floating-point constants are by default double-precision, and if they're then operated on along with a single-precision variable the value of the variable gets promoted to double precision, then the operation gets done, and then often the result gets converted back to single precision again. This is obviously silly, so I've used Clang's "-Wdouble-promotion" to find instances of this and mark the constants as single-precision as well. This is a bit awkward for PI, which ends up with a cast. Maybe there should be a PIF, or maybe PI should just be single-precision. This doesn't eliminate all warnings from -Wdouble-promotion. Some of the others might merit fixing but adding explicit casts to double just to shut the compiler up would be going too far, I feel.
* Miscellaneous const fixesBen Harris2023-02-18
| | | | | These are cases where -Wcast-qual complained and the only change needed was to add or remove a "const" (or sometimes an entire cast).
* Use unreserved macro names for multiple-include protectionBen Harris2023-02-18
| | | | | | | Some headers used macros named like _THING_H for multiple-include protection. That style of name is reserved in ISO C, though, so I've replaced it with PUZZLES_THING_H which is my favourite of the other styles in use.
* Replace a buch of "const static" with "static const"Ben Harris2023-02-18
| | | | C18 section 6.11.5 says that "const static" is obsolescent.
* Unequal: use %u to format an unsigned intBen Harris2023-02-18
|
* Undead: be a bit more careful about sprintf buffer sizesBen Harris2023-02-18
|
* Revert "Stop persistent-mode fuzzpuzz exiting prematurely"Ben Harris2023-02-18
| | | | | | | That was completely wrong: a "continue" at the end of the loop is unnecessary. This reverts commit b91f9824b6f73290051025317f3387c7212fa05f.
* Make emcc.c clean under -Wmissing-prototypes etcBen Harris2023-02-18
| | | | | | | | Also -Wstrict-prototypes and -Wmissing-variable-declarations. Functions that are called from JavaScript now have a separate declaration at the top of the file with a comment reminding one to update emcclib.js if they're changed.
* Mosaic: ignore taps above/left of the gridChris Boyle2023-02-18
| | | | | | | Thanks to Larry Hastings for the patch (tweaked to include drag/release). (cherry picked from Android port, commit 377e61b144c518a3d9efba66be08bf00ff6596e8)
* Stop persistent-mode fuzzpuzz exiting prematurelyBen Harris2023-02-18
| | | | In the transition to fuzz_one() I'd lost a "continue".
* Support Honggfuzz's persistent mode in fuzzpuzzBen Harris2023-02-18
| | | | | | | | Unlike AFL, Honggfuzz's compiler wrapper doesn't provide a convenient preprocessor macro, so we have to have CMake detect the existence of HF_ITER. Also the resulting program can't run outside of Honggfuzz, so maybe some additional cleverness is called for there as well. Still, it makes Honggfuzz go ten times faster, which is nice.
* Use -Wmissing-prototypes with GCC as wellBen Harris2023-02-18
| | | | | -Wmissing-prototypes was what I wanted all along, but somehow I'd mis-read the documentation and thought it wasn't.
* Buildscr: include a test build with clang + STRICT.Simon Tatham2023-02-18
| | | | | | I've just enabled a warning that only fires in that mode, so we need to keep running the build in that configuration to ensure further instances of the warning aren't introduced.
* Fix missing statics and #includes on variables.Simon Tatham2023-02-18
| | | | | | | | | | | | | | | | | | | | | | | After Ben fixed all the unwanted global functions by using gcc's -Wmissing-declarations to spot any that were not predeclared, I remembered that clang has -Wmissing-variable-declarations, which does the same job for global objects. Enabled it in -DSTRICT=ON, and made the code clean under it. Mostly this was just a matter of sticking 'static' on the front of things. One variable was outright removed ('verbose' in signpost.c) because after I made it static clang was then able to spot that it was also unused. The more interesting cases were the ones where declarations had to be _added_ to header files. In particular, in COMBINED builds, puzzles.h now arranges to have predeclared each 'game' structure defined by a puzzle backend. Also there's a new tiny header file gtk.h, containing the declarations of xpm_icons and n_xpm_icons which are exported by each puzzle's autogenerated icon source file and by no-icon.c. Happily even the real XPM icon files were generated by our own Perl script rather than being raw xpm output from ImageMagick, so there was no difficulty adding the corresponding #include in there.
* Fix unused variable warnings from clang.Simon Tatham2023-02-18
| | | | | | | | If you enable -DSTRICT=ON in cmake and also build with clang, it reports a couple of variables set but not otherwise used. One was genuinely unused ('loop_found' in loop_deductions in Loopy); the other is used by debug statements that are usually but not always compiled out.
* Add -Wmissing-prototypes to STRICT clang builds.Simon Tatham2023-02-18
| | | | | | | | Ben added -Wmissing-declarations in commit 3a3e491a8bc624e for gcc builds, and observed that clang's option of the same name doesn't do the same job. But clang does _have_ an option to do the same job: it's just spelled differently. Added -Wmissing-prototypes in clang builds, so that those will check the same thing.
* Enable -Wmissing-declarations in STRICT mode on GCCBen Harris2023-02-18
| | | | | | | Clang's -Wmissing-declarations means something quite different, so we only enable it on GCC. This is slightly silly since Clang's -Wmissing-declarations is enabled by default, but it makes it clear that we know they're different.
* Mark many more function (and some objects) staticBen Harris2023-02-18
| | | | | | | | | | | | | I noticed commit db3b531e2cab765a00475054d2e9046c9d0437d3 in the history where Simon added a bunch of "static" qualifiers. That suggested that consistently marking internal functions "static" is desirable, so I tried a build using GCC's -Wmissing-declarations, which requires prior declaration (presumed to be in a header file) of all global functions. This commit makes the GTK build clean under GCC's -Wmissing-declarations. I've also adding "static" to a few obviously internal objects, but GCC doesn't complain about those so I certainly haven't got them all.
* Call deallocate() in matching.c test routinesBen Harris2023-02-18
| | | | | | | This is mostly so that the function is used at all, but I've also removed another memory leak from --autotest mode to make it apparently leak-free. The testing from standard input mode has more leaks than I want to fix.
* Adjust fuzzpuzz sample shell commands to not include "/*"Ben Harris2023-02-16
| | | | | | GCC warns about that character sequence in a comment. I shouldn't have assumed that having only edited a comment meant I could get away without a test build.
* Tracks: set drag_s{x,y} even if starting off-gridChris Boyle2023-02-16
| | | | | | | | | | | Otherwise, if subsequent mouse/finger movement lines up with the previous drag attempt's start, then suddenly a drag is in progress from there, which is confusing. Fixes #588 (cherry picked from Android port, commit 8ce1bbe460d70a915caf2dbeb30354d22dc8a8ef)
* Update and expand comment at the head of fuzzpuzzBen Harris2023-02-16
| | | | | It now correctly describes what fuzzpuzz does. It also provides an example of how to use it with AFL++.
* Separate fuzzing and harness in fuzzpuzzBen Harris2023-02-16
| | | | | | | | | There's now a function, fuzz_one(), that processes a single save file, and main() arranges to call this a suitable number of times depending on whether we're in AFL persistent mode or not. This makes things a bit cleaner, and will probably make adding good support for other fuzzers, or just switching entirely to the horrible but popular libFuzzer interface, simpler.
* js: Hide type menu if there's only one preset and no configurationBen Harris2023-02-16
| | | | It seems a bit silly to display it when there's only one option.
* Solo: cope with pencil marks when tilesize == 1Ben Harris2023-02-16
| | | | | | | | | | | | Solo's layout calculations for pencil marks could fail with a tilesize of 1, generating an assertion failure: "draw_number: Assertion `pbest > 0' failed." This was reported as Debian bug #905852. My solution is slightly silly, namely to change a ">" in the test for whether a new layout is the best so far to ">=". This allows for finding a (terrible) layout even for tilesize == 1, and also has the side-effect of slightly preserring wide layouts over tall ones. Personally, I think that's an improvement.
* Note in the documentation that Pattern clues are in orderBen Harris2023-02-15
| | | | Requested in Debian bug #642207 and seems perfectly reasonable.
* Tighten grid-size limit in MinesBen Harris2023-02-15
| | | | | | | | | | | | | | | | | | | | | | Mines uses random_upto() to decide where to place mines, and random_upto() takes a maximum limit of 2^28-1, so limit the number of grid squares to that (or INT_MAX if someone's still trying to build on a 16-bit system). This avoids an assertion failure: "random_upto: Assertion `bits < 32' failed." which can be demonstrated by this save file: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection VERSION :1:1 GAME :5:Mines PARAMS :5:18090 CPARAMS :5:18090 DESC :11:r9,u,MEdff6 UI :2:D0 TIME :1:0 NSTATES :1:2 STATEPOS:1:2 MOVE :4:O2,1
* Make sure that moves in Flood use only valid coloursBen Harris2023-02-14
| | | | | | | | | | | | | | | | | | | If execute_move() receieves a move that uses a colour beyond the range for the current game, it now rejects it. Without this a solve string containing an invalid colour would cause an assertion failure: "fill: Assertion `oldcolour != newcolour' failed." While I was in the area I put a range check on colours for normal moves as well. To demonstrate the problem, load this save file: SAVEFILE:41:Simon Tatham's Portable Puzzle Collection VERSION :1:1 GAME :5:Flood PARAMS :7:6x6c6m5 CPARAMS :7:6x6c6m3 DESC :39:432242034203340350204502505323231342,17 NSTATES :1:2 STATEPOS:1:2 MOVE :2:S6
* Fix over-long lines in devel.butBen Harris2023-02-13
| | | | Most of these are my fault anyway.
* More validation of solve moves in FloodBen Harris2023-02-13
| | | | | | | | To avoid assertion failures while painting it, we need to ensure that the purported solution in a solve move doesn't include filling with the current top-left colour at any point. That means checking the first entry against the current top-left colours, and each later one against its predecessor.
* Validate that save file values are ASCII (mostly)Ben Harris2023-02-13
| | | | | | | | | | | | | | Apart from "SEED" records, all values in save files generated by Puzzles should be printable ASCII. This is enforced by assertion in the saving code. However, if a save file with non-ASCII move strings (for instance) manages to get loaded then these non-ASCII values can cause an assertion failure on saving. Instead, the loading code now checks values for ASCIIness. This will not only avoid problems when re-saving files, but will also defend the various internal parsers from at least some evil strings. It shouldn't invalidate any save files actually generated by Puzzles, but it will sadly invalidate some of my fuzzing corpus.
* Extend fuzzpuzz to test more codeBen Harris2023-02-13
| | | | | | Now if the input save file loads correctly, fuzzpuzz asks the back-end to draw the puzzle. All the drawing operations are no-ops, but this tests the drawing code in the back-end.
* Reserialise the game in fuzzpuzzBen Harris2023-02-13
| | | | | This means that the serialising code gets tested, and also provides a convenient way to canonicalise a (valid) save file.
* Avoid division by zero in Cube grid-size checksBen Harris2023-02-13
| | | | | | | | On a triangular grid, Cube allows either d1 or d2 (but not both) to be zero, so it's important to check that each one is not zero before dividing by it. The crash could be triggered by, for instance "cube t0x2".
* Mosaic: don't duplicate the description being validatedBen Harris2023-02-13
| | | | | | Mosaic's validate_desc() doesn't write to the description string, so it has no need to make a copy of it. And if it doesn't copy it, it can't leak the copy.
* Loopy: free the grid description string if it's invalidBen Harris2023-02-13
|
* Twiddle: don't read off the end of parameter strings ending 'm'Ben Harris2023-02-13
| | | | | The overrun could be demonstrated by specifying a parameter string of "3x3m" to a build with AddressSanitizer.
* Free new game_state properly in Mosaic's execute_move()Ben Harris2023-02-13
| | | | | | Using sfree() rather than free_game() in the error paths meant that various arrays referenced from the game_state weren't properly freed. Also one error path didn't free the game_state at all.
generated by cgit v1.1 at 2026-05-04 18:38:13 +0000