Ticket #239 (reopened defect)

Opened 9 months ago

Last modified 7 months ago

makedepend gets confused with multiple #include "filename..." in same program

Reported by: caa Owned by: caa
Priority: normal Milestone:
Component: MSVC makefiles Version: SVN trunk
Severity: major Keywords:
Cc: olly, charlie Blocked By:
Operating System: Microsoft Windows Blocking:

Description (last modified by olly) (diff)

Test condition:

In foo.cpp:

#include "bar.hpp"
#include "test1.hpp"

In test1.cpp:

#include "bar.hpp:

In test2.cpp:

#include "bar.hpp"

In test1.hpp:

#include "bar.hpp"

In bar.hpp:

#include "caa1.hpp"
#include "caa2.hpp"

When makedepend is processing file test1.hpp, it does not detect that bar.hpp has already been included. It DOES however, detect the fact that #include files in bar.hpp have already been processed.

Resultant output: There are two anomolies here 1. bar.hpp appears twice in the list of dependencies for foo.o (low priority) 2. All subsequent files (to makedepend) that #include bar.hpp do NOT specify files included by bar.hpp in their dependency list (high priority)

For example:

The makedepend command line has been supplied source filenames of foo.cpp, test1.cpp, and test2.cpp

The resultant makedepend output looks something like this:

foo.o: bar.hpp caa1.hpp caa2.hpp test1.hpp bar.hpp 
test1.o: bar.hpp
test2.o: bar.hpp

I am actively debugging the code to see if I can fix the bug. Any help would be appreciated. I am using MSVC (I ported the project file to VS 2003).

Change History

Changed 9 months ago by olly

  • status changed from new to assigned
  • component changed from Build system to MSVC makefiles
  • severity changed from critical to major

This bug is in the MSVC makefiles, not the build system.

And I don't see how this rates as "critical" which is defined as "crashes, loss of data, severe memory leak", so marking as "major" instead.

The makedepend implementation which the MSVC makefiles use is very poor. Another problem is that it apparently doesn't handle "#if defined ..." (we don't seem to have a bug for that).

And worst of all, it has buffer overrun problems:

http://xapian.org/cgi-bin/bugzilla/show_bug.cgi?id=196

I'm not at all happy that we're still pointing people to a set of makefiles which include such code, but I'm not a Windows guy, so there's not a lot I can do about it short of removing the link from the download page, which seems a bit extreme. Sadly Charlie doesn't seem to care.

I'd suggest a good short-term fix for all these problems is to find a better makedepend implementation. Something in Perl would be a good choice as we already require Perl and it rules out buffer overflow issues. Richard and I have both had a quick look, but didn't find a perl one which could just be slotted in.

The best long-term fix is to sort things out so MSVC can be used with the standard build system, so we wouldn't have to maintain two parallel build systems:

http://xapian.org/cgi-bin/bugzilla/show_bug.cgi?id=169

Changed 9 months ago by caa

Fair enough...but the buffer-overflow problem does seem unrelated to this bug. The buffer overflow can be corrected with the #define (or better yet, porting to C and using the STL string class)

Changed 9 months ago by olly

I'm not saying it is related as such.

Just noting that the current makedepend implementation has multiple flaws, so replacing is probably a better option than trying to fix them all. Just fixing this one flaw would be wasted effort if we then replace this implementation to address the other flaws.

I don't see how a #define can fix the buffer overflow bug. We could enlarge the buffer if that's what you mean, but there's still the potential to overflow it - just feed it a long enough string for the path.

And sure we could port to C++ and use std::string, but there are numerous "make depend" implementations out there, so picking an existing one which isn't so buggy would almost certainly be significantly less work.

Changed 9 months ago by caa

  • owner changed from newbugs to caa
  • status changed from assigned to new

I know its academic (as this utility will not be maintained), but I do wish to point out that the problem which is the subject of this bug is a "logic error" and string sizes / memory allocation notwithstanding, this is an algorithmic problem.

In my particular case, switching to a Perl version of the tool is going to require some serious refactoring of my makefiles.

That said, I'm going to "give it a go" and try to fix this bug...

Changed 9 months ago by caa

  • status changed from new to assigned

Changed 9 months ago by caa

  • status changed from assigned to closed
  • resolution set to fixed

In file include.c, starting at line 268: There is a compound 'if' statement that checks to see if the #include file is already in the list (via strcmp) _AND_ make sure the flags do not have the INCLUDED_SYM flag set.

I don't know why the flag enters into the mix - apparently, this has to do with a #define variable affecting the inclusion of subordinate #include files.

At any rate, reducing the 'if' statement to the strcmp test only appears to fix the problem.

What's the best way to update the code (or should I just make the patch valid for my version of the software)?

Comment: The code appears to use a lot of arcane techniques (IMHO) and its not very easy to follow. Sadly, the clever design is overshadowed by the sloppy implementation and lack of comments.

Changed 8 months ago by olly

  • cc charlie@…, olly@… added
  • status changed from closed to reopened
  • resolution deleted

Comment: The code appears to use a lot of arcane techniques (IMHO) and its not very easy to follow. Sadly, the clever design is overshadowed by the sloppy implementation and lack of comments.

You seem to have assigned this bug to yourself, which is fine if you're working on it, but that does mean that none of the Xapian developers have seen more recent updates to it as we don't currently have a mailing list for bug update messages. I happened to notice this bug while checking through bug reports while preparing a release. The state shouldn't be "RESOLVED FIXED" though, as this bug is still present in the makedepend in our tree.

In case it's not clear, we didn't write this makedepend utility - it's a piece of third party code found on the internet. I haven't studied most of the code in detail, so you probably have more idea than me how it works judging from the detail in your questions. You could try asking the original author(s) if you need clarification. If you're confident you have a suitable fix, Charlie can actually test it.

I certainly do agree that this code it has its problems - hence my belief that it's going to be simpler to replace it with an alternative implementation than to try to fix them all...

Changed 8 months ago by trac

  • platform set to Microsoft Windows

Changed 7 months ago by olly

  • description modified (diff)

(Description wiki-formatted)

caa: Did you come up with a suitable patch for Charlie to try?

Note: See TracTickets for help on using tickets.