Gentoo Forums
Gentoo Forums
Gentoo Forums
Quick Search: in
Issue with C program only during emerge process w/ GCC 4.6
View unanswered posts
View posts from last 24 hours

 
Reply to topic    Gentoo Forums Forum Index Portage & Programming
View previous topic :: View next topic  
Author Message
nitro322
Guru
Guru


Joined: 24 Jul 2002
Posts: 594
Location: USA

PostPosted: Fri Jan 11, 2013 4:56 am    Post subject: Issue with C program only during emerge process w/ GCC 4.6 Reply with quote

I'm rebuilding my desktop after GCC 4.6 hit stable (I know it's not necessary, but I want to take advantage of the noew corei7 optimizations) and hit a problem with an out-of-tree ebuild I maintain. This ebuild has the following prepare function:

src_prepare() {
# Unpack and rebuild GRP file in proper format
unpackssi "${CDROM_ROOT}/dukedc/dukedcpp.ssi" \
|| die "unpack SSI file failed"
kgroup dukedc.grp *.*
}

(...which I just realized should probably be done in src_unpack(), but that's a different topic)

The issue is with that unpackssi command. unpackssi is a simple C binary that I've installed via a different, dedicated ebuild. It's used to unpack files from a non-standard archive format.

unpackssi builds under both GCC 4.5 and 4.6, and runs fine under both when manually calling it. However, when the above command is called during an emerge, it fails. I can't figure out why, and I'm hoping a portage guru can help me figure out why I get different results when this is run as part of an ebuild vs. when I run it manually.

To illustrate, here's the error from unpackssi I get during an emerge:

Code:
>>> Preparing source in /tmp/portage/games-fps/duke3d-dukedc-1.0/work ...
unpackssi - .SSI File Unpacker
Copyright (c) 2003 Jonathon Fowler
This software is distributed under the terms of the GNU General Public License.

File is SSI Version 1
Error: Failed allocating memory for file index.
 * ERROR: games-fps/duke3d-dukedc-1.0 failed (prepare phase):
 *   unpack SSI file failed


However, as mentioned, I can CD into the work directory and manually run the same command with no problem:

Code:
blackdog work # unpackssi "/home/user/data/apps/games/PC/Media/Duke Nukem 3D (Atomic Edition)/Duke it Out in D.C./CD/dukedc/dukedcpp.ssi"
unpackssi - .SSI File Unpacker
Copyright (c) 2003 Jonathon Fowler
This software is distributed under the terms of the GNU General Public License.

File is SSI Version 1
File:           /home/jbreland/data/apps/games/PC/Media/Duke Nukem 3D (Atomic Edition)/Duke it Out in D.C./CD/dukedc/dukedcpp.ssi
Package Title:  DUKE IT OUT IN D.C.

Unpacking CINEOV3.ANM (2858 bytes)...done
<SNIP>


Also, if I rebuild/reinstall unpackssi with GCC 4.5, it'll work fine even during the above emerge process.

Here's the relevant section of code from unpackssi.c, in case it helps:

Code:
        filenames = (struct file *)malloc(sizeof(struct file) * numfiles);
        if (!filenames) {
            fclose(fp);
            puts("Error: Failed allocating memory for file index.");
            return -2;
        }


So... any ideas? I know a bit of C, but that filenames line is way out of my league. Plus, the fact that this works manually, but fails through portage, is really throwing me for a loop.

Thanks. Also, I know I included a lot of info above, so please let me know if anything's not clear and I'll be happy to explain a little better.
_________________
https://www.legroom.net/
Back to top
View user's profile Send private message
Hu
Moderator
Moderator


Joined: 06 Mar 2007
Posts: 21489

PostPosted: Sat Jan 12, 2013 1:18 am    Post subject: Reply with quote

It sounds like some sort of environment variable is relevant to whether it fails. Could you modify the C program to print out the value of errno immediately after failure, and to print the value of numfiles? You can use this block this in lieu of the block you posted:
Code:
        filenames = (struct file *)malloc(sizeof(struct file) * numfiles);
        if (!filenames) {
            fprintf(stderr, "Error: Failed allocating memory for file index of %u files of %u bytes each: %s.\n", numfiles, sizeof(struct file), strerror(errno));
            fclose(fp);
            return -2;
        }
You may need to adjust the format string if you use very strict warnings, but this should print the information we need. I suspect that the product of sizeof(struct file) * numfiles exceeds the size you can allocate.
Back to top
View user's profile Send private message
nitro322
Guru
Guru


Joined: 24 Jul 2002
Posts: 594
Location: USA

PostPosted: Sun Jan 13, 2013 3:31 am    Post subject: Reply with quote

Hi, Hu. Thanks for the suggestion. Just spent a little while playing with this, and have a few things to report:

1. The change you suggested didn't work as-is. gcc first complained about error being undefined, so after doing a bit of searching I found that I can just include errno.h to solve that. It then compiled, but gave me a segmentation fault instead of any meaningful output. That segfault appears to be coming from strerror(errno), and I don't know how to fix that one.

2. If I remove that last bit from printf, I do get some output. Here's what it returns:

Error: Failed allocating memory for file index of 16 files of 24 bytes.

16 files is the correct count of packed files. Not sure where 24 bytes comes from, though.

3. I actually did another complete rebuild (had some issues with other packages as well, including some changed USE flags, that was causing separate issues), and now I get this error with unpackssi regardless of whether I run it manually or within an ebuild. Don't know why it's behaving different now, and I'm certain I verified that I could run it manually when compiled with 4.6. Only guess is that maybe since gcc 4.6 itself was recompiled with 4.6 this time around, it's doing something a bit different? Wild guess, of course, but don't know what else it could be.

With that last change, this is probably more appropriate to the Unsupported Software section, since it no longer appears to be an issue with portage, so if any mods wish to move this, please feel free. Either way, though, I'd certainly appreciate any additional suggestions. :-)
_________________
https://www.legroom.net/
Back to top
View user's profile Send private message
Hu
Moderator
Moderator


Joined: 06 Mar 2007
Posts: 21489

PostPosted: Sun Jan 13, 2013 5:46 am    Post subject: Reply with quote

Something very strange is happening if you cannot allocate 16 structures of 24 bytes each. Do you have any ulimit settings enabled? What other resources does the program allocate before this point?
Back to top
View user's profile Send private message
nitro322
Guru
Guru


Joined: 24 Jul 2002
Posts: 594
Location: USA

PostPosted: Sun Jan 13, 2013 7:08 am    Post subject: Reply with quote

I'm pretty sure I don't have any ulimit settings beyond what gentoo provides by default. Have never needed to mess with that on my desktop before.

As for what else the program is doing, since you seem to be far more competent at C than I, it'd probably be both easier and faster to take a look at the original program than have me try to interpret what it's doing. It's actually a pretty straightforward, single file program <200 lines long. If you'd care to take a peek, you can grab it from here: http://www.jonof.id.au/misc. It's unpackssi.zip.

Again. thanks for the assistance with this.
_________________
https://www.legroom.net/
Back to top
View user's profile Send private message
Hu
Moderator
Moderator


Joined: 06 Mar 2007
Posts: 21489

PostPosted: Sun Jan 13, 2013 6:52 pm    Post subject: Reply with quote

That Makefile could use some work. I would replace it with this:
Makefile:
unpackssi: unpackssi.c
Then, tc-export CC to get the right compiler. The default GNU make rules will handle passing your CFLAGS/LDFLAGS in the right place.

The source file is unfortunately endian-dependent, but since you did not report a problem with that, we can assume you are on the right endianness. Are you building this on a 64-bit machine? If yes, then I think I see a problem:
unpackssi.c:
    28      long numfiles;
    79         fread(&numfiles, 4, 1, fp);
   100         filenames = (struct file *)malloc(sizeof(struct file) * numfiles);
Previously, I said you may need to adjust the format string I gave you. Apparently, that was correct. You definitely need to adjust the format string, and if you had, the error, although not its root cause, would have been obvious. Since numfiles is declared as long, on an amd64 machine, it will be 8 bytes. The low 4 are filled from the file. The high 4 are undefined, and may well contain a value that results in an allocation failure. Since the format string I gave you asked for %u, a plain unsigned, the high 4 bytes were dropped during formatting, so the output looked correct. Consider how the program would work if the declaration of numfiles was instead long numfiles = 0x00ff00ff00ff00ff;. Since the value is not initialized explicitly, its starting value is whatever was on the stack before, so my proposed initialization is a possible, although unlikely, value. If it had the value I propose, then that fread would change it to 0x00ff00ff00000010, where the underlined portion is the region changed by the fread and the upper half is unchanged. Such an allocation would then be huge, and would fail. However, when passed to fprintf, the %u causes it to read only the underlined portion, discarding the upper portion, so the output says 16, as you expected it should. If you had changed the format string, it would instead have printed the entire 8 bytes, revealing the problem.

Since this program is wrong on amd64, it is probably a happy accident that gcc 4.5 compiled it in a way that worked for you. The quick way to get this working is to force it to build as an x86 program by passing -m32. The correct way is to patch the code to use integers of the proper size and/or to ensure that the variables are fully initialized to safe starting values before loading SSI file data into the low part of the variable.
Back to top
View user's profile Send private message
nitro322
Guru
Guru


Joined: 24 Jul 2002
Posts: 594
Location: USA

PostPosted: Tue Jan 15, 2013 5:55 am    Post subject: Reply with quote

Hu, great analysis. Thanks for really digging into this. I hadn't even considered the 32-bit thing, but that makes perfect sense; after all, this program was last touched in 2003, before (consumer) 64-bit was anywhere near common.

So with your suggestions in mind, I found a rather simple fix that seems to work great: just replace all 'long' declarations with 'int'. According to some documentation I found, 'int' is 4 bytes under both 32- and 64-bit platforms, so this should make it work on 64-bit as intended, and not change the behavior at all on 32-bit platforms. Did a quick test and it works great; even verified it works with those other ebuilds that call unpackssi that caused the original problem I reported.

And yeah, really weird it worked like it did under GCC 4.5.

So, two questions:

1. Given your more extensive background here, does this seem like a reasonable solution to you? or could I be in a similar situation the next time something in the compiler changes?

2. Separately, how exactly do I use tc-export? I can't find any documentation on it, and searching through ebuilds for examples didn't really clear it up. I just added this to my ebuild:

Code:
src_compile() {
    tc-export CC
    emake || die "..."
}


...which is similar to the usage in most of the other ebuilds I found, but emake still calles 'gcc' directly, as specified in the original Makefile. I'm sure I'm missing something obvious here, but I'd appreciate any pointers. Only thing immediately coming to mind is updating the Makefile to reference the $CC env var, but I can't figure out how to actually do that in a Makefile.
_________________
https://www.legroom.net/
Back to top
View user's profile Send private message
Hu
Moderator
Moderator


Joined: 06 Mar 2007
Posts: 21489

PostPosted: Wed Jan 16, 2013 3:23 am    Post subject: Reply with quote

nitro322 wrote:
Hu, great analysis. Thanks for really digging into this. I hadn't even considered the 32-bit thing, but that makes perfect sense; after all, this program was last touched in 2003, before (consumer) 64-bit was anywhere near common.

So with your suggestions in mind, I found a rather simple fix that seems to work great: just replace all 'long' declarations with 'int'. According to some documentation I found, 'int' is 4 bytes under both 32- and 64-bit platforms, so this should make it work on 64-bit as intended, and not change the behavior at all on 32-bit platforms.

1. Given your more extensive background here, does this seem like a reasonable solution to you? or could I be in a similar situation the next time something in the compiler changes?
That should work. It would be clearer to use a specific type such as int32_t from stdint.h, which should be a 32-bit signed integer if it is defined at all. It would only be undefined on very exotic systems where 32-bit integers are completely unavailable.
nitro322 wrote:
2. Separately, how exactly do I use tc-export? I can't find any documentation on it, and searching through ebuilds for examples didn't really clear it up. I just added this to my ebuild:

Code:
src_compile() {
    tc-export CC
    emake || die "..."
}


...which is similar to the usage in most of the other ebuilds I found, but emake still calles 'gcc' directly, as specified in the original Makefile. I'm sure I'm missing something obvious here, but I'd appreciate any pointers. Only thing immediately coming to mind is updating the Makefile to reference the $CC env var, but I can't figure out how to actually do that in a Makefile.
That is necessary, but insufficient. The Makefile in the zip you referenced overrides $CC, so setting it in the environment is ineffective. You can either change the Makefile to the one I proposed, which relies heavily on default values and so would respect $CC, or you can use instead emake CC=$(tc-getCC). Variables specified on the make command line override variables specified in the Makefile, so such a definition would force make to use your value of CC.
Back to top
View user's profile Send private message
nitro322
Guru
Guru


Joined: 24 Jul 2002
Posts: 594
Location: USA

PostPosted: Sat Jan 19, 2013 2:23 am    Post subject: Reply with quote

Ah, I misunderstood what you had said previously about the makefile. I thought you meant change that one line to remove the .exe from the resulting binary (which I had already done). I didn't realize you meant make that one line the entire file. Doing that worked a treat. Thanks also for pointing out the tc-getCC command. I won't need it here, but I'm sure that'll be useful later.

Also switched over to using int32_t as recommended. That's another new one for me.

Really appreciate all the help, and I think I'm good now. Thanks, Hu!
_________________
https://www.legroom.net/
Back to top
View user's profile Send private message
Display posts from previous:   
Reply to topic    Gentoo Forums Forum Index Portage & Programming All times are GMT
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum