Review Request: Fix read beyond stream end in rpp
David Nolden
zwabel+reviewboard at gmail.com
Thu Dec 3 08:35:07 UTC 2009
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2315/#review3375
-----------------------------------------------------------
Ship it!
The patch looks correct. There is definitely something wrong with that code. Unfortunately it looks like we don't have a test-case for it, else that test-case wouldn fail on this.
Just make sure that cppcodecompletiontest, cppexpressionparsertest, and duchaintest still succeed, as they also contain some tests for the preprocessor.
- David
On 2009-12-03 07:13:25, Hamish Rodda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2315/
> -----------------------------------------------------------
>
> (Updated 2009-12-03 07:13:25)
>
>
> Review request for KDevelop.
>
>
> Summary
> -------
>
> I encountered an invalid read whilst running kdevelop under valgrind. The attached patch would seem to fix the issue, but I wanted to check if it was semantically correct before comitting (as we don't have an automated test for the preprocessor).
>
> Valgrind error:
> ==25012== Thread 9:
> ==25012== Invalid read of size 4
> ==25012== at 0x2A0B8C17: rpp::Stream::operator==(char) const (pp-stream.h:103)
> ==25012== by 0x2A0B5FEC: rpp::pp_macro_expander::operator()(rpp::Stream&, rpp::Stream&) (pp-macro-expander.cpp:295)
> ==25012== by 0x2A0B7E9C: rpp::pp_macro_expander::operator()(rpp::Stream&, rpp::Stream&) (pp-macro-expander.cpp:568)
> ==25012== by 0x2A0B7E9C: rpp::pp_macro_expander::operator()(rpp::Stream&, rpp::Stream&) (pp-macro-expander.cpp:568)
> ==25012== by 0x2A0BFD53: rpp::pp::operator()(rpp::Stream&, rpp::Stream&) (pp-engine.cpp:261)
> ==25012== by 0x2A0BF134: rpp::pp::processFileInternal(QString const&, QByteArray const&, QVector<unsigned int>&) (pp-engine.cpp:98)
> ==25012== by 0x2A0BEFE5: rpp::pp::processFile(QString const&, QByteArray const&) (pp-engine.cpp:85)
> ==25012== by 0x29E0EC89: PreprocessJob::run() (preprocessjob.cpp:233)
> ==25012== by 0x29DFFC1C: CPPParseJob::parseForeground() (cppparsejob.cpp:203)
> ==25012== by 0x29E11385: PreprocessJob::sourceNeeded(QString&, rpp::Preprocessor::IncludeType, int, bool) (preprocessjob.cpp:565)
> ==25012== by 0x2A0BF92D: rpp::pp::handle_include(bool, rpp::Stream&, rpp::Stream&) (pp-engine.cpp:193)
> ==25012== by 0x2A0BF294: rpp::pp::handle_directive(unsigned int, rpp::Stream&, rpp::Stream&) (pp-engine.cpp:131)
> ==25012== Address 0x2e692ed8 is 0 bytes after a block of size 24 alloc'd
> ==25012== at 0x4C25153: malloc (vg_replace_malloc.c:195)
> ==25012== by 0x2A0B4820: QVector<unsigned int>::malloc(int) (qvector.h:382)
> ==25012== by 0x2A0B42FD: QVector<unsigned int>::QVector(int) (qvector.h:388)
> ==25012== by 0x2A0B302A: rpp::Stream::Stream(unsigned int const*, unsigned int, rpp::Anchor const&, rpp::LocationTable*) (pp-stream.cpp:92)
> ==25012== by 0x2A0B7E4D: rpp::pp_macro_expander::operator()(rpp::Stream&, rpp::Stream&) (pp-macro-expander.cpp:566)
> ==25012== by 0x2A0B7E9C: rpp::pp_macro_expander::operator()(rpp::Stream&, rpp::Stream&) (pp-macro-expander.cpp:568)
> ==25012== by 0x2A0BFD53: rpp::pp::operator()(rpp::Stream&, rpp::Stream&) (pp-engine.cpp:261)
> ==25012== by 0x2A0BF134: rpp::pp::processFileInternal(QString const&, QByteArray const&, QVector<unsigned int>&) (pp-engine.cpp:98)
> ==25012== by 0x2A0BEFE5: rpp::pp::processFile(QString const&, QByteArray const&) (pp-engine.cpp:85)
> ==25012== by 0x29E0EC89: PreprocessJob::run() (preprocessjob.cpp:233)
> ==25012== by 0x29DFFC1C: CPPParseJob::parseForeground() (cppparsejob.cpp:203)
> ==25012== by 0x29E11385: PreprocessJob::sourceNeeded(QString&, rpp::Preprocessor::IncludeType, int, bool) (preprocessjob.cpp:565)
>
>
> This addresses bugs 214298 and potentially.
> https://bugs.kde.org/show_bug.cgi?id=214298
> https://bugs.kde.org/show_bug.cgi?id=potentially
>
>
> Diffs
> -----
>
> trunk/extragear/sdk/kdevelop/languages/cpp/parser/rpp/pp-macro-expander.cpp 1056447
>
> Diff: http://reviewboard.kde.org/r/2315/diff
>
>
> Testing
> -------
>
> I executed 'pp' on each test file, which appeared to work. I haven't yet run the same full execution of kdevelop under valgrind with the change to confirm that it works.
>
>
> Thanks,
>
> Hamish
>
>
More information about the KDevelop-devel
mailing list