[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gnash-commit
Subject:    [Gnash-commit] /srv/bzr/gnash/trunk r11181: Fix some segfaults due
From:       Benjamin Wolsey <bwy () benjaminwolsey ! de>
Date:       2009-06-29 13:07:38
Message-ID: E1MLHPl-0006Kb-2n () sv ! gnu ! org
[Download RAW message or body]

------------------------------------------------------------
revno: 11181
committer: Benjamin Wolsey <bwy@benjaminwolsey.de>
branch nick: trunk
timestamp: Mon 2009-06-29 15:07:38 +0200
message:
  Fix some segfaults due to executing ABC tags that aren't completely parsed.
modified:
  libcore/parser/abc_block.cpp
  libcore/swf/DoABCTag.h
    ------------------------------------------------------------
    revno: 11179.1.2
    committer: Benjamin Wolsey <bwy@benjaminwolsey.de>
    branch nick: work
    timestamp: Mon 2009-06-29 14:24:55 +0200
    message:
      Failure to parse classes will leave invalid pointers, which tend to
      cause segfaults. Fixes to class parsing may prevent this situation, or
      resizing the class container on failure, but for now we will not
      execute any ABC block where parsing failed.
    modified:
      libcore/swf/DoABCTag.h
    ------------------------------------------------------------
    revno: 11179.1.3
    committer: Benjamin Wolsey <bwy@benjaminwolsey.de>
    branch nick: work
    timestamp: Mon 2009-06-29 14:27:22 +0200
    message:
      Minor cleanups.
    modified:
      libcore/parser/abc_block.cpp

["r11181.diff" (r11181.diff)]

=== modified file 'libcore/parser/abc_block.cpp'
--- a/libcore/parser/abc_block.cpp	2009-06-29 11:36:39 +0000
+++ b/libcore/parser/abc_block.cpp	2009-06-29 12:27:22 +0000
@@ -910,14 +910,12 @@
 	boost::uint32_t count = _stream->read_V32();
 	log_abc("There are %u instances.", count);
 	_classes.resize(count);
-	for (unsigned int i = 0; i < count; ++i)
-	{
-		asClass *pClass;
+	for (size_t i = 0; i < count; ++i) {
+		asClass* pClass;
 		//Read multiname index.
 		boost::uint32_t index = _stream->read_V32();
 		// 0 is allowed as a name, typically for the last entry.
-		if (index >= _multinamePool.size())
-		{
+		if (index >= _multinamePool.size()) {
 			log_error(_("ABC: Out of bounds instance name."));
 			return false;
 		}
@@ -961,11 +959,14 @@
 			if (!pSuper)
 			{
 				log_error(_("ABC: Super type not found (%s), faking."), 
-					_stringTable->value(_multinamePool[super_index].getABCName()));
+					_stringTable->value(
+                        _multinamePool[super_index].getABCName()));
+
 				// While testing, we will add a fake type, rather than abort.
 				pSuper = mCH->newClass();
 				pSuper->setName(_multinamePool[super_index].getABCName());
-				mCH->getGlobalNs()->addClass(_multinamePool[super_index].getABCName(), pSuper);
+				mCH->getGlobalNs()->addClass(
+                        _multinamePool[super_index].getABCName(), pSuper);
 				// return false;
 			}
 
@@ -996,26 +997,21 @@
                 _stringPool[_multinamePool[index].getABCName()],
                 super_index, flags | 0x0);
 
-		if (flags & INSTANCE_SEALED)
-			pClass->setSealed();
-		if (flags & INSTANCE_FINAL)
-			pClass->setFinal();
-		if (flags & INSTANCE_INTERFACE)
-			pClass->setInterface();
-		if ((flags & 7) == INSTANCE_DYNAMIC)
-			pClass->setDynamic();
+		if (flags & INSTANCE_SEALED) pClass->setSealed();
+		if (flags & INSTANCE_FINAL) pClass->setFinal();
+		if (flags & INSTANCE_INTERFACE) pClass->setInterface();
+		if ((flags & 7) == INSTANCE_DYNAMIC) pClass->setDynamic();
 
-		if (flags & INSTANCE_PROTECTED_NS) // Protected Namespace
-		{
+		if (flags & INSTANCE_PROTECTED_NS) {
 			boost::uint32_t ns_index = _stream->read_V32();
-			if (ns_index >= _namespacePool.size())
-			{
+			if (ns_index >= _namespacePool.size()) {
 				log_error(_("ABC: Out of bounds namespace for protected."));
 				return false;
 			}
 			// Set the protected namespace's parent, if it exists.
 			if (pClass->getSuper()->hasProtectedNs())
-				_namespacePool[ns_index]->setParent(pClass->getSuper()->getProtectedNs());
+				_namespacePool[ns_index]->setParent(
+                        pClass->getSuper()->getProtectedNs());
 			pClass->setProtectedNs(_namespacePool[ns_index]);
 		}
 
@@ -1023,13 +1019,11 @@
 		// implement. They must be interfaces, and they must exist.
 		boost::uint32_t intcount = _stream->read_V32();
 		log_abc("This instance has %u interfaces.", intcount);
-		for (unsigned int j = 0; j < intcount; ++j)
-		{
+		for (size_t j = 0; j < intcount; ++j) {
 			boost::uint32_t i_index = _stream->read_V32();
 			log_abc("Interface %u has multiname index=%u", i, i_index);
 			// 0 is allowed as an interface, typically for the last one.
-			if (i_index >= _multinamePool.size())
-			{
+			if (i_index >= _multinamePool.size()) {
 				log_error(_("ABC: Out of bounds name for interface."));
 				return false;
 			}
@@ -1042,13 +1036,13 @@
 			}
 			pClass->pushInterface(pInterface);
 		}
+
 		// The next thing should be the constructor.
 		// TODO: What does this mean exactly? How does it differ from the one in
 		// the class info block?
 		boost::uint32_t offset = _stream->read_V32();
 		log_abc("Moffset: %u", offset);
-		if (offset >= _methods.size())
-		{
+		if (offset >= _methods.size()) {
 			log_error(_("ABC: Out of bounds method for initializer."));
 			return false;
 		}
@@ -1082,16 +1076,17 @@
 	log_abc("Begin reading classes.");
 	boost::uint32_t count = _classes.size();
 	log_abc("There are %u classes.", count);
-	for (unsigned int i = 0; i < count; ++i)
-	{
+	
+    for (size_t i = 0; i < count; ++i) {
 		asClass* pClass = _classes[i];
 		boost::uint32_t offset = _stream->read_V32();
 		log_abc("Class %u(%s) static constructor index=%u", i, pClass, offset);
-		if (offset >= _methods.size())
-		{
+
+        if (offset >= _methods.size()) {
 			log_error(_("ABC: Out of bound static constructor for class."));
 			return false;
 		}
+
 		// Don't validate for previous owner.
 		pClass->setStaticConstructor(_methods[offset]);
 
@@ -1102,14 +1097,13 @@
 		
 		boost::uint32_t tcount = _stream->read_V32();
 		log_abc("This class has %u traits.", tcount);
-		for (unsigned int j = 0; j < tcount; ++j)
-		{
+		for (size_t j = 0; j < tcount; ++j) {
 			Trait &aTrait = newTrait();
 			aTrait.set_target(pClass, true);
 			if (!(aTrait.read(_stream, this)))
 				return false;
 		}
-	} // end of classes loop
+	} 
 	return true;
 }
 
@@ -1292,30 +1286,41 @@
 	if (!read_integer_constants()) return false;
 	if (!read_unsigned_integer_constants()) return false;
 	log_abc("Done reading unsigned integer constants.");
-	if (!read_double_constants()) return false;
+	
+    if (!read_double_constants()) return false;
 	log_abc("Done reading double constants.");
-	if (!read_string_constants()) return false;
+	
+    if (!read_string_constants()) return false;
 	log_abc("Done reading string constants.");
-	if (!read_namespaces()) return false;
+	
+    if (!read_namespaces()) return false;
 	log_abc("Done reading namespaces.");
-	if (!read_namespace_sets()) return false;
+	
+    if (!read_namespace_sets()) return false;
 	log_abc("Done reading namespace sets.");
-	if (!read_multinames()) return false;
+	
+    if (!read_multinames()) return false;
 	log_abc("Done reading multinames.");
-	if (!read_method_infos()) return false;
+	
+    if (!read_method_infos()) return false;
 	log_abc("Done reading method infos.");
-	if (!skip_metadata()) return false;
+	
+    if (!skip_metadata()) return false;
 	log_abc("Done reading metadata.");
-	if (!read_instances()) return false;
+	
+    if (!read_instances()) return false;
 	log_abc("Done reading instances.");
-	if (!read_classes()) return false;
+	
+    if (!read_classes()) return false;
 	log_abc("Done reading classes.");
-	if (!read_scripts()) return false;
+	
+    if (!read_scripts()) return false;
 	log_abc("Done reading scripts.");
-	if (!read_method_bodies()) return false;
+	
+    if (!read_method_bodies()) return false;
 	log_abc("Done reading stuff.");
 
-	for(unsigned int i=0;i<_methods.size();i++) {
+	for (size_t i=0; i < _methods.size(); ++i) {
 		log_abc("Method %d body:", i);
 		IF_VERBOSE_PARSE(_methods[i]->print_body());
 	}

=== modified file 'libcore/swf/DoABCTag.h'
--- a/libcore/swf/DoABCTag.h	2009-06-15 11:32:49 +0000
+++ b/libcore/swf/DoABCTag.h	2009-06-29 12:24:55 +0000
@@ -50,10 +50,10 @@
 		Machine *mach = vm.getMachine();
 		as_object* global = vm.getGlobal();
 		
-        mABC->prepare(mach);
+        _abc->prepare(mach);
 
 		log_debug("Begin execute abc_block.");
-		mach->initMachine(mABC, global);
+		mach->initMachine(_abc, global);
 		log_debug("Executing machine...");
 		mach->execute();
 	}
@@ -87,10 +87,15 @@
 			in.read_string(name);
 		}
 
-		abc_block* block = new abc_block();
-		block->read(in);
-        // mABC = block;
-		DoABCTag* ABCtag = new DoABCTag(block);
+        std::auto_ptr<abc_block> block(new abc_block());
+		if (!block->read(in)) {
+            log_error("ABC parsing error while processing DoABCTag. This "
+                    "tag will never be executed");
+            return;
+        }
+
+        // _abc = block;
+		DoABCTag* ABCtag = new DoABCTag(block.release());
 		
 		IF_VERBOSE_PARSE (
             log_parse(_("tag %d: DoABCDefine"), tag);
@@ -102,9 +107,9 @@
 
 private:
 
-	DoABCTag(abc_block *block) : mABC(block) {}
+	DoABCTag(abc_block* block) : _abc(block) {}
 
-	abc_block *mABC;
+	abc_block* _abc;
 	
 };
 



_______________________________________________
Gnash-commit mailing list
Gnash-commit@gnu.org
http://lists.gnu.org/mailman/listinfo/gnash-commit


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic