bugGnash - The GNU Flash player - Bugs: bug #22816, failures in libamf.all testsuite

 
 

bug #22816: failures in libamf.all testsuite

Submitted by:  Sandro Santilli <strk>
Submitted on:  Wed 02 Apr 2008 10:52:57 PM UTC  
 
Category: coreSeverity: 5 - Blocker
Release: NoneStatus: Fixed
Privacy: PublicAssigned to: Rob Savoye <rsavoye>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Wed 30 Apr 2008 10:23:47 AM UTC, comment #11:

valgrind-clean runs after your last commit.
I guess we need more tests here ;)

Sandro Santilli <strk>
Project Member
Mon 07 Apr 2008 08:28:24 AM UTC, comment #10:

test_sol fails in another place for me, but no abort.
Results are arbitrary as code relies on uninitialized
memory:

==4110== Invalid read of size 2
==4110== at 0x40C7313: amf::AMF::extractVariable(unsigned char*) (amf.cpp:1218)
==4110== by 0x40E708C: amf::SOL::readFile(std::string&) (sol.cpp:377)
==4110== by 0x804EE0C: test_read(std::string&) (test_sol.cpp:113)
==4110== by 0x8050237: main (test_sol.cpp:95)
==4110== Address 0x4714376 is 302 bytes inside a block of size 303 alloc'd
==4110== at 0x4022F14: operator new[](unsigned) (vg_replace_malloc.c:268)
==4110== by 0x40E6F2D: amf::SOL::readFile(std::string&) (sol.cpp:336)
==4110== by 0x804EE0C: test_read(std::string&) (test_sol.cpp:113)
==4110== by 0x8050237: main (test_sol.cpp:95)
==4110==
==4110== Conditional jump or move depends on uninitialised value(s)
==4110== at 0x40C733B: amf::AMF::extractVariable(unsigned char*) (amf.cpp:1221)
==4110== by 0x40E708C: amf::SOL::readFile(std::string&) (sol.cpp:377)
==4110== by 0x804EE0C: test_read(std::string&) (test_sol.cpp:113)
==4110== by 0x8050237: main (test_sol.cpp:95)
==4110==
==4110== Invalid read of size 1
==4110== at 0x40C7340: amf::AMF::extractVariable(unsigned char*) (amf.cpp:1222)
==4110== by 0x40E708C: amf::SOL::readFile(std::string&) (sol.cpp:377)
==4110== by 0x804EE0C: test_read(std::string&) (test_sol.cpp:113)
==4110== by 0x8050237: main (test_sol.cpp:95)
==4110== Address 0x4714378 is 1 bytes after a block of size 303 alloc'd
==4110== at 0x4022F14: operator new[](unsigned) (vg_replace_malloc.c:268)
==4110== by 0x40E6F2D: amf::SOL::readFile(std::string&) (sol.cpp:336)
==4110== by 0x804EE0C: test_read(std::string&) (test_sol.cpp:113)
==4110== by 0x8050237: main (test_sol.cpp:95)
==4110==
==4110== Invalid read of size 1
==4110== at 0x804E3BF: test_write(std::string&) (test_sol.cpp:219)
==4110== by 0x805029C: main (test_sol.cpp:99)
==4110== Address 0x4715c80 is 0 bytes after a block of size 0 alloc'd
==4110== at 0x4022F14: operator new[](unsigned) (vg_replace_malloc.c:268)
==4110== by 0x40D1D9E: amf::Buffer::init(unsigned) (buffer.cpp:36)
==4110== by 0x40D1DF3: amf::Buffer::Buffer(unsigned) (buffer.cpp:65)
==4110== by 0x40D8E5E: amf::Element::init(std::string const&, std::string const&) (element.cpp:226)
==4110== by 0x804DAC1: test_write(std::string&) (test_sol.cpp:217)
==4110== by 0x805029C: main (test_sol.cpp:99)

Sandro Santilli <strk>
Project Member
Mon 07 Apr 2008 07:35:46 AM UTC, comment #9:

robustness issue.. that's an element w/out a name.
I guess we could add some intentionally malformed sol files
in our testsuite to catch these kind of errors.

Rob, what's the problem with using check_equals interface
rather then direct TestRun.{fail,pass} ?
We have the check.h abstracting use of dejagnu when available
or custom handling when not, with check_equals also reporting
the actually obtained result for better debugging (which
is how I found out what the problem was..)

Sandro Santilli <strk>
Project Member
Mon 07 Apr 2008 05:45:38 AM UTC, comment #8:

Segfaults are indeed fixed for me.

It's still possible to get aborts, though, when Gnash tries to access old SOL files. SOL files written since the segfaults were fixed seem to cause no problems. I attach an old one that causes this failure when running the youtube player2.swf from the command line.

46978172424272] 07:34:56 SECURITY: Opening SharedObject file: /home/benjamin/.gnash/videostats.sol
46978172424272] 07:34:56 DEBUG: /home/benjamin/.gnash/videostats.sol is an SOL file
46978172424272] 07:34:56 DEBUG: AMF element length is: 60636
46978172424272] 07:34:56 DEBUG: Got unprintable characters for the element name!
46978172424272] 07:34:56 DEBUG: AMF element name is:
46978172424272] 07:34:56 DEBUG: Read 1 AMF objects from /home/benjamin/.gnash/videostats.sol
46978172424272] 07:34:56 DEBUG: Unexpected exception from swf_function execution: basic_string::_S_construct NULL not valid
46978172424272] 07:34:56 DEBUG: Unexpected exception from swf_function execution: basic_string::_S_construct NULL not valid
46978172424272] 07:34:56 DEBUG: Unexpected exception from swf_function execution: basic_string::_S_construct NULL not valid
terminate called after throwing an instance of 'std::logic_error'
what(): basic_string::_S_construct NULL not valid

That is with youtube, for instance - it happens as soon as Gnash tries to access a pre-existing SOL file.

To reproduce: put videostats.sol into the standard SOL directory; run "gnash http://youtube.com/player2.swf"

(file #15411)

Benjamin Wolsey <bwy>
Project Member
Sat 05 Apr 2008 09:42:42 PM UTC, comment #7:

No, it means there was a bug in calculating the byte offset for the next element. This is fixed in HEAD now.

Rob Savoye <rsavoye>
Project AdministratorIn charge of this item.
Thu 03 Apr 2008 09:01:22 AM UTC, comment #6:

test_sol.cpp agrees there'a single element in settings.sol:

FAILED: nelems == '1' (expected: 10)

Sandro Santilli <strk>
Project Member
Thu 03 Apr 2008 08:59:50 AM UTC, comment #5:

Now, for the test_sol, failures seem to arise to unexpected
number of elements in the settings.sol file.
soldump against it gives me:

Will use "/home/strk/src/gnash/gnash-head/testsuite/libamf.all/settings.sol" for sol files location
SOL file "/home/strk/src/gnash/gnash-head/testsuite/libamf.all/settings.sol" read in
Dumping SOL file
The file name is: /home/strk/src/gnash/gnash-head/testsuite/libamf.all/settings.sol
The size of the file is: 302
The name of the object is: settings
gain: 50 ( 00 00 00 00 00 00 49 40 )

Does that mean there's a single element in it ?

Sandro Santilli <strk>
Project Member
Thu 03 Apr 2008 08:55:10 AM UTC, comment #4:

The attached patch (element_initstring.patch) fixes the short allocation and thus the test_el run (valgrind-clear after that).

(file #15393)

Sandro Santilli <strk>
Project Member
Thu 03 Apr 2008 08:50:02 AM UTC, comment #3:

The failure in test_el is due to a memory corruption.
Running under valgrind the test fails (as memory is shaked)
but valgrind tells what the problem is:

==8411== Invalid read of size 1
==8411== at 0x40239D3: strlen (mc_replace_strmem.c:242)
==8411== by 0x8053ADA: std::string::compare(char const*) const (char_traits.h:257)
==8411== by 0x804FA42: main (basic_string.h:2151)
==8411== Address 0x4712adc is 0 bytes after a block of size 12 alloc'd
==8411== at 0x4022F14: operator new[](unsigned) (vg_replace_malloc.c:268)
==8411== by 0x40D1E9E: amf::Buffer::init(unsigned) (buffer.cpp:36)
==8411== by 0x40D1EF3: amf::Buffer::Buffer(unsigned) (buffer.cpp:65)
==8411== by 0x80584CB: amf::Element::init(std::string const&, std::string const&) (element.cpp:244)
==8411== by 0x805857B: amf::Element::init(std::string const&) (element.cpp:233)
==8411== by 0x804F847: main (test_el.cpp:96)

Sandro Santilli <strk>
Project Member
Thu 03 Apr 2008 08:47:45 AM UTC, comment #2:

The attached patch (libamftest.patch) makes the test_sol.cpp
and test_el.cpp files consisten with the rest of the testsuite,
which is using the 'check.h' header and the macro therein.
With this, you can have more info of what fails:

--=[ ./testsuite/libamf.all ]=--
FAIL: test_el: el.to_string() == 'Hello World!!' (expected: Hello World!) [/home/strk/src/gnash/gnash-head/testsuite/libamf.all/test_el.cpp:98]
FAIL: test_sol: nelems > 9 [/home/strk/src/gnash/gnash-head/testsuite/libamf.all/test_sol.cpp:118]
FAIL: test_sol: *el->getData() == '/' (expected: 0) [/home/strk/src/gnash/gnash-head/testsuite/libamf.all/test_sol.cpp:207]
FAIL: test_sol: *el->getData() == 'p' (expected: 0) [/home/strk/src/gnash/gnash-head/testsuite/libamf.all/test_sol.cpp:217]
FAIL: test_sol: *el->getData() == 'p' (expected: 0) [/home/strk/src/gnash/gnash-head/testsuite/libamf.all/test_sol.cpp:280]

Note that the line numbers are those after applying the patch...

(file #15392)

Sandro Santilli <strk>
Project Member
Wed 02 Apr 2008 10:57:56 PM UTC, comment #1:

Just a note: these results are with all my patches in bug #22814
and bug #22806 applied.

Without those patches (current head) I get a segfault and
a failure, as reported here:
http://lists.gnu.org/archive/html/gnash-dev/2008-04/msg00000.html

Sandro Santilli <strk>
Project Member
Wed 02 Apr 2008 10:52:57 PM UTC, original submission:

Running /home/strk/src/gnash/gnash-head/testsuite/libamf.all/../simple.exp ...
FAIL: test_el: Created string element
FAIL: test_sol: Read SOL file
FAIL: test_sol: defaultcamea set
FAIL: test_sol: localSecPath set

=== Summary ===

# of expected passes 28
# of unexpected failures 4

Sandro Santilli <strk>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #15411:  videostats.sol added by bwy (85B - application/octet-stream)
file #15393:  element_initstring.patch added by strk (508B - text/x-patch)
file #15392:  libamftest.patch added by strk (15KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bwy (Updated the item)
  • -unavailable- added by rsavoye (Posted a comment)
  • -unavailable- added by strk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 30 Apr 2008 10:23:47 AM UTCstrkStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Mon 07 Apr 2008 05:45:38 AM UTCbwyAttached File-=>Added videostats.sol, #15411
    Sat 05 Apr 2008 09:42:42 PM UTCrsavoyeStatusNone=>Ready For Test
    Thu 03 Apr 2008 08:55:10 AM UTCstrkAttached File-=>Added element_initstring.patch, #15393
    Thu 03 Apr 2008 08:47:45 AM UTCstrkAttached File-=>Added libamftest.patch, #15392

    Back to the top


    Powered by Savane 3.1-cleanup1