After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 681825 - index.qhp has malformed ref, qhelpgenerator errors out.
index.qhp has malformed ref, qhelpgenerator errors out.
Status: RESOLVED FIXED
Product: doxygen
Classification: Other
Component: general
1.8.2
Other Windows
: Normal critical
: ---
Assigned To: Dimitri van Heesch
Dimitri van Heesch
: 684789 688652 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-08-14 10:43 UTC by Bastiaan
Modified: 2012-12-26 16:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case. (56.64 KB, application/x-zip)
2012-08-15 20:17 UTC, Bastiaan
  Details
patch to fix reference to freed memory (591 bytes, patch)
2012-09-12 00:13 UTC, Matthew Woehlke
none Details | Review

Description Bastiaan 2012-08-14 10:43:34 UTC
Hi,

Since 1.8.2 the ref="" part of the main page in <toc> is not well formed anymore. Here is a diff around the problematic line in index.qhp:

   <filterSection>
     <filterAttribute>doxygen</filterAttribute>
     <toc>
-      <section title="PIAS Manual 2012" ref="index.html">
-        <section title=" " />
+      <section title="PIAS Manual 2012" ref="°">
         <section title="PIAS: Introduction" ref="index.html" />

I have also seen ref="ø" so it looks like the variable that should contain "index.html" is not initialized and contains garbage.

Because of this qhelpgenerator errors out with
Error in line 1: Encountered incorrectly encoded content.

BTW, when qhelpgenerator is run by Doxygen automatically, you never get to see the error message.

Best regards,
Bastiaan.
Comment 1 Dimitri van Heesch 2012-08-15 19:02:12 UTC
Can you please attach a self-contained example (source+config file in a tar or zip) that allows me to reproduce the problem?

Note that the following has changed in qhp index:
http://doxygen.svn.sourceforge.net/viewvc/doxygen/trunk/src/qhp.cpp?r1=821&r2=829
(I could imagine that the change around line 129 may explain the bogus ref...)
Comment 2 Bastiaan 2012-08-15 20:17:55 UTC
Created attachment 221313 [details]
Test case.

I must admit that I haven't regenerated Doxygen files to see if there are required changes to the configuration for 1.8.2, I did that for 1.8.1.2. I can do that if you think it would make a difference.

Regarding line 129, I think it needs to be
(QCString("index")+Doxygen::htmlFileExtension).data()
or the like. I would expect the same for fullProjectname, don't know why that works as is. Maybe you shouldn't use a temporary?

Thanks,
Bastiaan
Comment 3 Matthew Woehlke 2012-09-12 00:01:07 UTC
Yes, you broke it in that commit. I'm suddenly being hit by this also, since upgrading to 1.8.2.

I do believe a temporary is required, otherwise the char* that is put in the array becomes a dangling pointer. (The .data() is implicit, but the QCString no longer exists after the array initializer is done, hence the pointer points at garbage. It's sheer luck if this ever works in its current form.)

Since this just broke my ability to generate documentation, I am probably going to try to patch it locally. If that works, I'll get back to you with a patch.
Comment 4 Matthew Woehlke 2012-09-12 00:11:44 UTC
FYI, valgrind says:

==30556== Invalid read of size 1
==30556==    at 0x611E50: convertToXML(char const*) (util.cpp:5354)
==30556==    by 0x5BC1BF: QhpXmlWriter::openPureHelper(char const*, char const* const*, bool) (qhpxmlwriter.cpp:124)
==30556==    by 0x5BC2AE: QhpXmlWriter::open(char const*, char const* const*) (qhpxmlwriter.cpp:56)
==30556==    by 0x5BAC14: Qhp::initialize() (qhp.cpp:132)
==30556==    by 0x43BEAC: IndexList::foreach(void (IndexIntf::*)()) (index.h:63)
==30556==    by 0x429CEC: generateOutput() (index.h:130)
==30556==    by 0x40443D: main (main.cpp:38)
==30556==  Address 0xe818380 is 0 bytes inside a block of size 11 free'd
==30556==    at 0x4A079AE: free (vg_replace_malloc.c:427)
==30556==    by 0x738710: QCString::~QCString() (scstring.cpp:81)
==30556==    by 0x5BABF2: Qhp::initialize() (qhp.cpp:131)
==30556==    by 0x43BEAC: IndexList::foreach(void (IndexIntf::*)()) (index.h:63)
==30556==    by 0x429CEC: generateOutput() (index.h:130)
==30556==    by 0x40443D: main (main.cpp:38)
==30556== 
==30556== Invalid read of size 1
==30556==    at 0x611EB1: convertToXML(char const*) (util.cpp:5354)
==30556==    by 0x5BC1BF: QhpXmlWriter::openPureHelper(char const*, char const* const*, bool) (qhpxmlwriter.cpp:124)
==30556==    by 0x5BC2AE: QhpXmlWriter::open(char const*, char const* const*) (qhpxmlwriter.cpp:56)
==30556==    by 0x5BAC14: Qhp::initialize() (qhp.cpp:132)
==30556==    by 0x43BEAC: IndexList::foreach(void (IndexIntf::*)()) (index.h:63)
==30556==    by 0x429CEC: generateOutput() (index.h:130)
==30556==    by 0x40443D: main (main.cpp:38)
==30556==  Address 0xe818381 is 1 bytes inside a block of size 11 free'd
==30556==    at 0x4A079AE: free (vg_replace_malloc.c:427)
==30556==    by 0x738710: QCString::~QCString() (scstring.cpp:81)
==30556==    by 0x5BABF2: Qhp::initialize() (qhp.cpp:131)
==30556==    by 0x43BEAC: IndexList::foreach(void (IndexIntf::*)()) (index.h:63)
==30556==    by 0x429CEC: generateOutput() (index.h:130)
==30556==    by 0x40443D: main (main.cpp:38)

(IOW, this is your friendly reminder that it is always good to run things under valgrind or another memory access checker every now and then :-). Ideally as part of your unit testing methodology, or at least pre-release validation.)
Comment 5 Matthew Woehlke 2012-09-12 00:13:56 UTC
Created attachment 224062 [details] [review]
patch to fix reference to freed memory

This seems to fix the problem on my system. Feel free to rework as you like to follow whatever conventions you desire, but this should give you the general idea.

(I also verified that it fixes the error reported by valgrind.)
Comment 6 Bastiaan 2012-09-12 07:27:16 UTC
(In reply to comment #3)

Thanks for the patch.

> I do believe a temporary is required, otherwise the char* that is put in the
> array becomes a dangling pointer.

That is actually what I was trying to say in the last sentence, in that QCString::operator+ returns a temporary object which is then discarded immediately (in the erroneous code) as it is not kept in scope. refFileName (in your patch) is a variable that you control the lifetime of, so it is less "temporary"; and of course the right thing to use.

Hope to see a new release soon :-)

Bastiaan.
Comment 7 Dimitri van Heesch 2012-09-16 12:04:36 UTC
Confirmed. Should be fixed in the next subversion update.
Comment 8 Dimitri van Heesch 2012-09-25 21:50:19 UTC
*** Bug 684789 has been marked as a duplicate of this bug. ***
Comment 9 Dimitri van Heesch 2012-11-19 18:57:36 UTC
*** Bug 688652 has been marked as a duplicate of this bug. ***
Comment 10 Dimitri van Heesch 2012-12-26 16:09:03 UTC
This bug was previously marked ASSIGNED, which means it should be fixed in
doxygen version 1.8.3. Please verify if this is indeed the case. Reopen the
bug if you think it is not fixed and please include any additional information
that you think can be relevant.