Dream Deaemon crash with Malloc()

ID:2291810
 
Sep 8 2017, 10:09 pm
Resolved
Look-behind assertions in regular expressions could cause a crash in some cases.
BYOND Version:511
Operating System:Linux
Web Browser:Firefox 55.0
Applies to:Dream Daemon
Status: Resolved (512.1386)
This issue has been resolved.
Descriptive Problem Summary:
After pulling an update from an repository into my machine and recompiling, Dream Daemon started throwing a crash with Malloc()
Numbered Steps to Reproduce Problem:
1. Use a Linux machine
2. Download the latest version of Paradise (Yes, it's SS13)
3. Compile it on the same machine
4. Run

Expected Results:
The server to run.
Actual Results:
Crash.
Does the problem occur:
Every time? Or how often?
Every time.
In other games?
No.
In other user accounts?
No.
On other computers?
No.
When does the problem NOT occur?
When I revert the merges.
Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? (Visit http://www.byond.com/download/build to download old versions for testing.)

Workarounds:
None. Only going back to the latest working version of Paradise, but that's no workaround.

Stack trace for the crash: https://hastebin.com/qavugevare.hs
No, I can't use any debugging tools for this. I'm on a VPS with a terminal-only interface.
Sep 8 2017, 11:28 pm
I can run some traces on this Monday. First I need to know which exact 511 build you're using, because your trace didn't include that.

But I'm also curious what kinds of changes were in the code you updated.
Sep 8 2017, 11:32 pm
511.1385 (happened in 137x as well, x being any number between 0 and 9)
And about the changes, you'll have to check the Paradise repository - I just pulled the changes from it. AFAIK, Dream Daemon shouldn't crash because of errors in DM, so I don't think it's the programmer's fault, but I could be wrong.
Sep 8 2017, 11:45 pm
nopm do you know around what day you had pulled changes from it before that point. It would help narrow down a git bisect.
Sep 9 2017, 12:20 am
The bisect will actually be quite big, since I pulled changes from Paradise into my own working repo with my own changes. Well, I stopped updating at March 30th. And I updated today...
Sep 9 2017, 2:00 am
so first I would confirm it happens to current base paradise code. if so, your changes don't matter.

There have been 1,925 commits since march 30th.

That would only require 11 git bisect commands to find the commit that created the bug.
Sep 9 2017, 2:15 am
Yep, cloned the Paradise repo and tested it. Exact same crash.
Sep 9 2017, 10:02 pm
It's a known issue with our codebase, Paradise- Something we've done at some point in the last two years or so completely destroyed Linux compatibility. We get everything from these weird malloc errors to the rsc file being so corrupted from compilation that the server can't run. We suspect it is at least in part related to inconsistencies with our folder case naming (some things contain spaces and upper case letters), but that isn't the only issue.
Sep 9 2017, 10:06 pm
This is the first time I've experienced issues with Paradise itself on my VPS. Like I said before, I had a repo with commits from March 30th that worked just fine, but pulling in the latest commits from Paradise to update my own version (I run a server with my own modifications) caused it to crash. I've reverted the changes, but I'd very much like to fix these issues, since there's only 2 coders on our team (me included), so we depend heavily on Paradise PRs for features and fixes.
Sep 12 2017, 10:50 am
Bump.
Sep 12 2017, 1:11 pm
I can trace this to a regex() call. Can you show me relevant portions of the code that do regex operations?
Sep 12 2017, 4:48 pm
LOL
There are hundreds if not thousands of lines in the Paradise codebase that do Regex operations.
Sep 12 2017, 5:26 pm
it would be something on startup before/during world/New() running nopm.
Sep 12 2017, 6:11 pm
Hm... I can't think of anything. Maybe someone with more experience coding Paradise could help?
Sep 13 2017, 11:46 am
I have narrowed it down to after globals are inits but before world/New() is ran

ie, its map loading. so an atom that does regex on New()

That should be easy to find.
Sep 13 2017, 12:21 pm
 


/regex/New(...)
    world.log << "before regex([jointext(args, ", ")])"
    . = ..(arglist(args))
    world.log << "after regex([jointext(args, ", ")])"

/regex/Find(...)
    world.log << "before [src].Find([jointext(args, ", ")])"
    . = ..(arglist(args))
    world.log << "after [src].Find([jointext(args, ", ")])"

/regex/Replace(...)
    world.log << "before [src].Replace([jointext(args, ", ")])"
    . = ..(arglist(args))
    world.log << "after [src].Find([jointext(args, ", ")])"



before regex((?<!\\)\*(.*?)(?<!\\)\*, g)
BUG: Crashing due to an illegal operation!



var/static/regex/italics = new("(?<!\\\\)\\*(.*?)(?<!\\\\)\\*", "g")


https://github.com/ParadiseSS13/Paradise/pull/7266/files
Sep 13 2017, 1:34 pm
Found it! Thanks MSO for narrowing that down.

The problem is that the look-behind assertion is not allocating properly, an issue that didn't show up in testing. I strongly advise the Paradise codebase to stop using look-behinds until 512 is out. The bug will be fixed in the first 512 release.
Sep 13 2017, 4:34 pm
Thank you MrStonedOne for helping out. I appreciate it! And thanks Lummox, for working on the bug on 512.
Sep 25 2017, 12:27 pm
Lummox JR resolved issue with message:
Look-behind assertions in regular expressions could cause a crash in some cases.