Skip to content

UNALIGNED_MEMORY generates syntax error with va_arg #1705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chino opened this issue Oct 12, 2013 · 5 comments
Closed

UNALIGNED_MEMORY generates syntax error with va_arg #1705

chino opened this issue Oct 12, 2013 · 5 comments

Comments

@chino
Copy link
Contributor

chino commented Oct 12, 2013

#include <stdarg.h>
int main(void)
{
    va_list argp;
    va_arg(argp, char *);
}

Compiling with:

emcc -s VERBOSE=1 -s UNALIGNED_MEMORY=1 test.c -o test.js &&
node test.js

Returns:

 SyntaxError: Unexpected token ;
     at Module._compile (module.js:437:25)
     at Object.Module._extensions..js (module.js:467:10)
     at Module.load (module.js:356:32)
     at Function.Module._load (module.js:312:12)
     at Module.runMain (module.js:492:10)
     at process.startup.processNextTick.process._tickCallback (node.js:244:9)

Link to the full test.js output here https://gist.github.com/chino/0f0dc6ec8c0b2eaed098

This was running:

$ emcc --version
emcc (Emscripten GCC-like replacement) 1.6.4 (commit 0a229cf5a240a1ebf4ff5d0ebb501a286bc96202)
Copyright (C) 2013 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ clang --version
clang version 3.3 (trunk 173279)
Target: x86_64-apple-darwin12.4.0
Thread model: posix

$ uname  -a
Darwin Daquinos-MacBook-Pro-2.local 12.4.0 Darwin Kernel Version 12.4.0: Wed May  1 17:57:12 PDT 2013; root:xnu-2050.24.15~1/RELEASE_X86_64 x86_64 i386 MacBookPro8,2 Darwin
@dinibu
Copy link
Contributor

dinibu commented Oct 12, 2013

+1 : I have seen this too.
$ emcc --version
emcc (Emscripten GCC-like replacement) 1.6.4 ()
Copyright (C) 2013 the Emscripten authors (see AUTHORS.txt)
This is free and open source software under the MIT license.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ clang --version
clang version 3.2 (tags/RELEASE_32/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix

$ uname -a
Linux the-todd 2.6.18-348.16.1.el5 #1 SMP Wed Aug 21 04:00:25 EDT 2013 x86_64 x86_64 x86_64 GNU/Linux

@kripken
Copy link
Member

kripken commented Oct 14, 2013

Fixed on incoming.

Note though that UNALIGNED_MEMORY is very unrecommended. Why do you need it?

@kripken kripken closed this as completed Oct 14, 2013
@chino
Copy link
Contributor Author

chino commented Oct 14, 2013

I'm working on porting @ForsakenGame and in the short run switching this on will make it much easier to get some progress.

This is also affecting the port to Pandora ARM architecture so it's obviously something we'll eventually fix.

As of right now basically all file reading code in the game reads the entire file contents into a byte array that it then proceeds to iterate over by casting the pointer to different sizes. This causes problems when for instance you try to read a short out of an unaligned offset.

For example:

#include <stdio.h>
#include <stdint.h>

// imagine this is a file with data
static uint8_t data[] = {
    1,0,     // short = 1
    1,       // byte  = 1
    8,0      // short = 8 but instead returns 2049
             //           unless built with -s UNALIGNED_MEMORY=1
};

int main( void )
{
    // we read the file using pointers
    uint16_t    *   short_ptr;
    uint8_t     *   byte_ptr;

    short_ptr = (uint16_t *) data;      // view as short pointer
    printf("%d,",*short_ptr++);         // read a short

    byte_ptr = (uint8_t *) short_ptr;   // view as byte pointer
    printf("%d,",*byte_ptr++);          // read a byte

    short_ptr = (uint16_t *) byte_ptr;  // view as short pointer
    printf("%d\n", *short_ptr++);       // read a short
}

Also network related code casts the byte arrays to various structs which I haven't investigated yet but I assume it might also have the same alignment issues ?

Thanks for the fix though!

@kripken
Copy link
Member

kripken commented Oct 15, 2013

Ah, I see. Yeah, x86 code can easily be non-portable that way, and then it breaks on ARM, emscripten, etc. Use the SAFE_HEAP/CHECK_HEAP_ALIGN options to help find the relevant spots.

@chino
Copy link
Contributor Author

chino commented Oct 15, 2013

I have to use those sparingly because enabling all the checks makes it extremely slow! It takes about two minutes for the main initialization since it pre-loads all textures and models right at startup. On a modern system that takes less than a second. Especially with the updated HD textures I had to disable the gamma ramp / color key emulation for now since it would add another minute or so to the load time: https://github.com/ForsakenX/forsaken/blob/master/render_gl_shared.c#L131

Right now I'm just moving step by step and hoping to get something rendering then I'll take some inspiration from your work on BananaBread to try and improve the startup time.

I've also been so far sticking with the immediate mode backend so I can try and help add missing features to Emscripten before I try moving to the new shader backends which I'm sure would help with performance.

chino pushed a commit to chino/emscripten that referenced this issue Nov 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants