StanfordPL / x64asm

x86-64 assembler library
Apache License 2.0
465 stars 60 forks source link

Assembler segfaults because it doesn't reserve enough memory #238

Open Detry322 opened 7 years ago

Detry322 commented 7 years ago

I don't have a minimal bug reproducing program, but when doing something of the form:

x64asm::Function function;
x64asm::Assembler assembler;

assembler.start(function);

// ... lots of assembly here, mov for me seemed to trigger the segfault ...
assembler.mov(r10, Imm64{10L});

assembler.finish();

cout << function << endl;

Given enough instructions, this program will segfault. Maybe this is because the implementations used to write instructions don't call the reserve method?

eschkufz commented 7 years ago

Performance was really important when we first wrote the assembler, so I think that an undocumented assumption was that you would call reserve if you needed it, but we wouldn't force you to call it for every instruction if you didn't want to.

Looking back, this was probably premature optimization (or wishful thinking --- it's hard to see now how this could ever have been much of a bottleneck). A much cleaner implementation would have been to put the reserve check into the Function class (I think Function --- I don't know what it's called now --- the thing that holds the assembled buffer) and not bothered the user with it.

(I'm just musing --- I haven't looked at this code in a while, and everything I'm saying might no longer be true.)

On Mon, May 8, 2017 at 10:09 AM Jack Serrino notifications@github.com wrote:

I don't have a minimal bug reproducing program, but when doing something of the form:

x64asm::Function function; x64asm::Assembler assembler;

assembler.start(function);

// ... lots of assembly here, mov for me seemed to trigger the segfault ... assm.mov(r10, Imm64{10L});

assm.finish();

cout << function << endl;

Given enough instructions, this program will segfault. Maybe this is because the implementations used to write instructions don't call the reserve method?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/StanfordPL/x64asm/issues/238, or mute the thread https://github.com/notifications/unsubscribe-auth/AAm4rmUdI19WCL4P8rnrkjopSVcBNYvbks5r30xGgaJpZM4NUOyy .

yasyf commented 7 years ago

@eschkufz maybe worth it to add to the documentation? we can do a PR

stefanheule commented 7 years ago

Yeah, we'd be happy to accept a PR to add this to the documentation!