Attachment 200733 Details for Bug 353605 – Better patch which only upates rangelist using children also updating start and end addresses

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Better patch which only upates rangelist using children also updating start and end addresses

353605-rangelist.patch (text/plain), 8.43 KB, created by Daniel Thomas

on 2011-08-02 11:37:50 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Thomas

Created: 2011-08-02 11:37:50 EDT

Size: 8.43 KB

patch

obsolete

>From 5beb5482a1a623248f98efab4f77f27b420065b9 Mon Sep 17 00:00:00 2001
>From: thomasd <thomasd@broadcom.com>
>Date: Mon, 25 Jul 2011 10:51:15 +0100
>Subject: [PATCH 1/3] Do proper merging of rangelists
>
>Signed-off-by: Daniel Thomas <thomasd@broadcom.com>
>---
> .../cdt/debug/edc/internal/symbols/Scope.java      |   61 ++++++++++++++++++--
> 1 files changed, 56 insertions(+), 5 deletions(-)
>
>diff --git a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>index d404c3d..77070d6 100644
>--- a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>+++ b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>@@ -13,6 +13,7 @@ package org.eclipse.cdt.debug.edc.internal.symbols;
> import java.util.ArrayList;
> import java.util.Collection;
> import java.util.Collections;
>+import java.util.Iterator;
> import java.util.List;
> import java.util.SortedMap;
> import java.util.TreeMap;
>@@ -69,7 +70,7 @@ public abstract class Scope implements IScope {
> 		return (lowAddress == null || highAddress == null)
> 		|| (lowAddress.isZero() && highAddress.isZero())
> 		|| (lowAddress.getValue().longValue() == -1 && highAddress.isZero()); // TODO: remove this case
>-	}
>+	}//FIXME: looks like a bug: no test that lowAddress.equals(highAddress)
> 
> 	/**
> 	 * Return the list of non-contiguous ranges for this scope.
>@@ -287,8 +288,7 @@ public abstract class Scope implements IScope {
> 			{
> 				if (rangeList != null) {
> 					if (scope.getRangeList() != null) {
>-						// TODO: merge properly
>-						rangeList = null;
>+						rangeList = mergeRangeLists(rangeList,scope.getRangeList());
> 					} else {
> 						((RangeList)rangeList).addLowRange(scope.getLowAddress().getValue().longValue());
> 					}
>@@ -298,8 +298,7 @@ public abstract class Scope implements IScope {
> 			if (scope.getHighAddress() != null && scope.getHighAddress().compareTo(highAddress) > 0) {
> 				if (rangeList != null) {
> 					if (scope.getRangeList() != null) {
>-						// TODO: merge properly
>-						rangeList = null;
>+						rangeList = mergeRangeLists(rangeList,scope.getRangeList());
> 					} else {
> 						((RangeList)rangeList).addHighRange(scope.getHighAddress().getValue().longValue());
> 					}
>@@ -308,6 +307,58 @@ public abstract class Scope implements IScope {
> 			}	
> 		}
> 	}
>+	protected IRangeList mergeRangeLists(IRangeList first, IRangeList second){
>+		Iterator<IRangeList.Entry> firstIterator = first.iterator();
>+		Iterator<IRangeList.Entry> secondIterator = second.iterator();
>+		RangeList answer = new RangeList();
>+		IRangeList.Entry firstHead = null;
>+		IRangeList.Entry secondHead = null;
>+		while (firstIterator.hasNext() || secondIterator.hasNext()
>+				|| firstHead != null || secondHead != null){
>+			if (firstHead == null && firstIterator.hasNext()){
>+				firstHead = firstIterator.next();
>+			}
>+			if (secondHead == null && secondIterator.hasNext()){
>+				secondHead = secondIterator.next();
>+			}
>+			if (firstHead == null){
>+				if (secondHead != null){
>+					answer.addRange(secondHead.low, secondHead.high);
>+					secondHead = null;
>+				}// else we have no more work to do and will exit at the end of the loop
>+			} else {
>+				if (secondHead == null){
>+					answer.addRange(firstHead.low,firstHead.high);
>+					firstHead = null;
>+				} else {// both not null so work to do
>+					long low,high;
>+					if (firstHead.low <= secondHead.low){
>+						low = firstHead.low;
>+						if (firstHead.high < secondHead.low){// we are just using firstHead
>+							high = firstHead.high;
>+							firstHead = null;
>+						} else {// overlap so use both
>+							high = Math.max(firstHead.high, secondHead.high);
>+							firstHead = null;
>+							secondHead = null;
>+						}
>+					} else {
>+						low = secondHead.low;
>+						if (secondHead.high < firstHead.low){// just using secondHead
>+							high = secondHead.high;
>+							secondHead = null;
>+						} else { // overlap so use both
>+							high = Math.max(firstHead.high, secondHead.high);
>+							firstHead = null;
>+							secondHead = null;
>+						}
>+					}
>+					answer.addRange(low,high);
>+				}
>+			}
>+		}
>+		return answer;
>+	}
> 	
> 	protected void addLineInfoToParent(IScope scope) {
> 		IScope cu = parent;
>-- 
>1.7.0.2
>
>From db5c54273ec77f14c9cb91a78216b2fc0c24604a Mon Sep 17 00:00:00 2001
>From: thomasd <thomasd@broadcom.com>
>Date: Tue, 26 Jul 2011 09:17:41 +0100
>Subject: [PATCH 2/3] Fixup rangelist information when fixing up low an high addresses
>
>So that the rangelist is not erroneously null when the low and high addresses
>have been fixed so that they are no longer both 0.
>
>Signed-off-by: Daniel Thomas <thomasd@broadcom.com>
>---
> .../cdt/debug/edc/internal/symbols/Scope.java      |   16 ++++++++++++++--
> 1 files changed, 14 insertions(+), 2 deletions(-)
>
>diff --git a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>index 77070d6..6aa7117 100644
>--- a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>+++ b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/Scope.java
>@@ -233,6 +233,7 @@ public abstract class Scope implements IScope {
> 		// figure it out from the functions
> 		IAddress newLowAddress = Addr64.MAX;
> 		IAddress newHighAddress = Addr64.ZERO;
>+		IRangeList newRangeList = new RangeList();
> 		boolean any = false;
> 		
> 		for (IScope kid : getChildren()) {
>@@ -243,8 +244,15 @@ public abstract class Scope implements IScope {
> 			if (kid.hasEmptyRange()) {
> 				continue;
> 			}
>-			
>+
> 			if (kid.getLowAddress().compareTo(baseAddress) > 0) {
>+				IRangeList kidRanges = kid.getRangeList();
>+				if (kidRanges == null){// If it didn't have ranges it still has one range
>+					kidRanges = new RangeList();
>+					((RangeList)kidRanges).addRange(kid.getLowAddress().getValue().longValue(),kid.getHighAddress().getValue().longValue());
>+				}
>+				newRangeList = mergeRangeLists(newRangeList,kidRanges);
>+
> 				if (kid.getLowAddress().compareTo(newLowAddress) < 0) {
> 					newLowAddress = kid.getLowAddress();
> 					any = true;
>@@ -261,7 +269,11 @@ public abstract class Scope implements IScope {
> 			//System.out.println("Needed to fix up ranges for " + getName());
> 			lowAddress = newLowAddress; 
> 			highAddress = newHighAddress;
>-			rangeList = null;
>+			int entryCount = 0;
>+			for (@SuppressWarnings("unused") IRangeList.Entry entry : newRangeList){
>+				++entryCount;
>+			}
>+			rangeList = (entryCount > 1) ? newRangeList : null;
> 		} else {
> 			if (lowAddress == null) {
> 				lowAddress = highAddress = Addr32.ZERO;
>-- 
>1.7.0.2
>
>From 4c559423c4074aad4d203500913de03b9e61af8f Mon Sep 17 00:00:00 2001
>From: thomasd <thomasd@broadcom.com>
>Date: Tue, 26 Jul 2011 09:20:12 +0100
>Subject: [PATCH 3/3] Only return a non-null IRangeList in readRangeList if it is non-empty
>
>As null is used to represent a null IRangeList and returning a non-null empty
>one results in a zero length entry at 0x0.
>
>Signed-off-by: Daniel Thomas <thomasd@broadcom.com>
>---
> .../internal/symbols/dwarf/DwarfInfoReader.java    |    5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
>diff --git a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/dwarf/DwarfInfoReader.java b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/dwarf/DwarfInfoReader.java
>index 087a197..dfb8b40 100644
>--- a/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/dwarf/DwarfInfoReader.java
>+++ b/org.eclipse.cdt.debug.edc/src/org/eclipse/cdt/debug/edc/internal/symbols/dwarf/DwarfInfoReader.java
>@@ -1915,6 +1915,8 @@ public class DwarfInfoReader {
> 				 */
> 	
> 				RangeList list = new RangeList();
>+				//Whether the list actually got anything added - otherwise we should return null
>+				boolean listAddedTo = false;
> 	
> 				long base = 0;
> 				long start = data.getInt();
>@@ -1937,6 +1939,7 @@ public class DwarfInfoReader {
> 						// ignore bogus entries: GCC-E sometimes generates these buggily (for artifical non-inlined functions)
> 						if (base + start >= codeRanges.getLowAddress()) {
> 							list.addRange(base + start, base + end);
>+							listAddedTo = true;
> 						}
> 					}
> 					start = data.getInt();
>@@ -1944,7 +1947,7 @@ public class DwarfInfoReader {
> 					
> 				} while (true);
> 				
>-				return list;
>+				return (listAddedTo) ? list : null;
> 				
> 			} catch (Throwable t) {
> 				EDCDebugger.getMessageLogger().logError(DwarfMessages.DwarfInfoReader_RangeReadFailed, t);
>-- 
>1.7.0.2
>

Actions: View | Diff

Attachments on bug 353605: 200731 | 200733 | 200895