Opened 17 years ago

Closed 16 years ago

Last modified 15 years ago

#196 closed defect (fixed)

makedepend tool can't cope with long INCLUDE paths

Reported by: Charlie Hull Owned by: Charlie Hull
Priority: normal Milestone: 1.1.0
Component: MSVC makefiles Version: SVN trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Operating System: Microsoft Windows

Description (last modified by Olly Betts)

The port of makedepend we're using can't cope with include paths longer than 512 bytes - according to Paul Rudin: "A number of those command line args are copied around with strcpy where the buffers involved are BUFSIZ large. BUFSIZ seems to be only 512 (from stdio.h) on my system so it's pretty easy to get buffer overflows. That -I"..." string that caused the problem was more than 512 chars for example. "

This can cause buffer overrun errors in makedepend, if the Windows system has a particularly large INCLUDE path in its environment variables.

Change History (9)

comment:1 by Olly Betts, 17 years ago

Operating System: Microsoft Windows
Status: newassigned

comment:2 by Olly Betts, 17 years ago

BUFSIZ is an entirely inappropriate constant to use here. We could just bump up the buffer size, but I'm not really happy shipping even auxiliary code with known buffer overflows.

Richard said on IRC that this makedepend implementation is rather ropey and doesn't understand "#if defined" or something like that, so I think we're best off just using a different implementation.

A version in Perl would be suitable (and avoid the risk of buffer overflows). A quick poke on google code search reveals a few candidates:

http://www.google.com/codesearch?as_filename=makedepend.pl

The first one I get is this, which seems to use compiler output and support MSVC:

http://www.google.com/codesearch?hl=en&q=+file:makedepend.pl+show:ToaQXwvMSUA:-Oq4N_DHuj4:g0t6i_Btkak&sa=N&cd=1&ct=rc&cs_p=http://www.meadowy.org/meadow/dists/beta/Meadow-2.00b2-src.tar.bz2&cs_f=Meadow-2.00b2-src/nt/makedepend.pl#first

comment:3 by Olly Betts, 17 years ago

I had a go at trying to use the perl makedepend.pl script in comment#1, but it's not a drop-in replacement (notably because it processes one file at a time), and it's hellish trying to slot it in with only a slow build cycle via buildbot to test with so I gave up.

I wonder if it would be better just to fix bug#169.

comment:4 by Richard Boulton, 17 years ago

I had a go at making the perl one work and gave up after a few hours of making little progress, too.

I think bug #169 might well be the way to go - I've now got mingw installed on my windows server, so I'll have a go at that again soon.

comment:6 by Olly Betts, 16 years ago

Component: Build systemMSVC makefiles
Description: modified (diff)
Milestone: 1.1.0
Owner: changed from New Bugs to Charlie Hull
Status: assignednew

If you're going to stick with the makedepend implementation, can you please at least patch it to die if the buffer would overflow?

comment:7 by Charlie Hull, 16 years ago

I've made the following changes to makedepend:

  • use a new #defined constant for buffer size, currently set to 4k
  • attempted to check any string copies or similar that use the above to prevent overflows
  • added some #pragmas and a #include - makedepend.exe now builds without any warnings
  • added some extra flags to the invocation of makedepend during the Make process - this has prevented the warnings it was generating, it seems it does understand all the #if directives used in Xapian after all.

Of course #169 would be a better approach, but for now we have a cleaner solution for Windows using makedepend.

comment:8 by Richard Boulton, 16 years ago

Resolution: fixed
Status: newclosed

As Charlie says, this bug is now fixed, though the makedepend in use is still rather horrible. It works well enough for now, and further effort in this direction should be spent on making MSVC work with the standard build system. Marking as resolved.

comment:9 by Olly Betts, 16 years ago

Thanks for the patch.

I'd say "fixed" isn't exactly right (unless MSVC imposes a limit on the size of the INCLUDE path) since users could still hit this, but it's probably the closest option given the resolutions we have to choose from.

The "#if defined" problem was fixed by a patch from Charlie a while ago:

http://trac.xapian.org/changeset?new=9712@trunk%2Fxapian-maintainer-tools%2Fwin32msvc%2Fmakedepend%2Fifparser.c&old=8996@trunk%2Fxapian-maintainer-tools%2Fwin32msvc%2Fmakedepend%2Fifparser.c

Hmm, actually looking at the source around there, there's a case to handle "defined" in parse_value(), which should handle "!defined", since the '!' case recursively calls parse_value().

So that change doesn't seem to do anything. I suspect the warnings reported (by Paul?) were actually due to #239 and the above change got accidentally included with some other updates.

comment:10 by Olly Betts, 16 years ago

After checking with Richard on IRC, I removed the "!defined" special case a while ago.

I've also copied the trunk version of makedepend back to the 1.0 branch for 1.0.7.

Note: See TracTickets for help on using tickets.