Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

The preprocessor returns incorrect ranges of macros with certain combinations of headers #24654

Closed Quuxplusone closed 8 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR24655
Status RESOLVED INVALID
Importance P normal
Reported by Dimitar Dobrev (dpldobrev@protonmail.com)
Reported on 2015-09-01 12:13:56 -0700
Last modified on 2016-01-11 08:57:05 -0800
Version unspecified
Hardware PC Windows NT
CC akyrtzi@gmail.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments testcase-pp.zip (782996 bytes, application/zip)
Blocks
Blocked by
See also
Created attachment 14804
A test case

My name is Dimitar Dobrev and I develop a project that creates C# wrappers for
C++ code. We use Clang to parse C++ and we have this bug -
https://github.com/mono/CppSharp/issues/543 . In short, a function
(QAbstractSlider::wheelEvent) is reported as having macros above itself while
in fact it has none. You can see the headers involved at
http://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/accessible/complexwidgets.h
and
http://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qabstractslider.h
.

The project leader, Joao Matos, and I have been working on it for more than a
week now and we think there may be a bug in
PreprocessingRecord::getPreprocessedEntitiesInRange. There is a comment in that
code where it is acknowledged that endings may not be ordered. However, we
believe that unlike what the comments states, that order is actually important.
Furthermore, we have discovered that replacing getEnd() with getBegin() in
findBeginLocalPreprocessedEntity eliminates our bug. This is, however, just a
workaround because Argyrios Kyrtzidis was kind enough to tell us the change
breaks some of your tests. We thought it made sense because you use binary
search which requires elements to be sorted, and the list is, evident from
PreprocessingRecord::addPreprocessedEntity, sorted by beginnings and therefore -
 because of macro expansion - not necessarily by endings at the same time.

I have attached a test case generated by running Clang with -frewrite-includes.
Let me know if I can help further.
Quuxplusone commented 9 years ago

Attached testcase-pp.zip (782996 bytes, application/zip): A test case

Quuxplusone commented 9 years ago

Hello, any comment about this bug?

Quuxplusone commented 8 years ago

The same happens when parsing http://code.qt.io/cgit/qt/qtdeclarative.git/tree/src/quick/scenegraph/util/qsgtextureprovider.h. This is a real problem to us, could you please let me know how I can help further?

Quuxplusone commented 8 years ago

The bug also happens with QAbstractAudioOutput from http://code.qt.io/cgit/qt/qtmultimedia.git/tree/src/multimedia/audio/qaudiosystem.h.

Quuxplusone commented 8 years ago

It happens with QAbstractAudioInput from http://code.qt.io/cgit/qt/qtmultimedia.git/tree/src/multimedia/audio/qaudiosystem.h too.

Quuxplusone commented 8 years ago
The bug, or at least part of it, can be reproduced by parsing this simple piece:

#define Q_SIGNALS protected

class MacroExpandedToAccessBug
{
Q_SIGNALS:
    void signal() {}
};

The result is that signal() and the default members share the same
AccessSpecDecl pointer. However, this is wrong because at least the default
constructor in C++ is public, not protected (quotes from the C++ standard at
http://stackoverflow.com/a/32235317).
Quuxplusone commented 8 years ago

The bug, or at least the part of it exemplified by the C++ piece in my previous comment, occurs when using LLVM/Clang revisions 253162/253161 respectively.

Quuxplusone commented 8 years ago

It might be related that isImplicit() is false for all generated members when parsing the C++ sample.

Quuxplusone commented 8 years ago
It's great that you are reducing your inputs, but we are still missing a way
for us to reproduce your bug. Could you create a file that is using the
libclang API and compiles to a command-line executable that demonstrates the
bug you want to fix ?
This file along with the input header(s) should be self-contained, ideally we
would not need the windows SDK or all of Qt.
Quuxplusone commented 8 years ago

It turned out to be a bug in our code.