[EDIFTokenizer] Account for byte size of UTF-8 characters correctly (#…

…962) * [EDIFTokenizer] Account for byte size of UTF-8 characters correctly Signed-off-by: Eddie Hung <[email protected]> * Fix EDIFTokenizer.tokenTooLong() for tokens that wrap around Signed-off-by: Eddie Hung <[email protected]> * Update TestEDIFParser with new offsets for extra-stressful EDIF Signed-off-by: Eddie Hung <[email protected]> * Test to check for unicode string Signed-off-by: Eddie Hung <[email protected]> * Update src/com/xilinx/rapidwright/edif/EDIFTokenizer.java Signed-off-by: eddieh-xlnx <[email protected]> * Update DCP submodule Signed-off-by: Eddie Hung <[email protected]> * Bump copyright year Signed-off-by: Eddie Hung <[email protected]> * Bump copyright year Signed-off-by: Eddie Hung <[email protected]> --------- Signed-off-by: Eddie Hung <[email protected]> Signed-off-by: eddieh-xlnx <[email protected]>
Xilinx · Feb 29, 2024 · 60a5714 · 60a5714
1 parent 359043d
commit 60a5714
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 16 deletions.
diff --git a/src/com/xilinx/rapidwright/edif/EDIFTokenizer.java b/src/com/xilinx/rapidwright/edif/EDIFTokenizer.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2022, Xilinx, Inc.
- * Copyright (c) 2022, Advanced Micro Devices, Inc.
+ * Copyright (c) 2022, 2024, Advanced Micro Devices, Inc.
  * All rights reserved.
  *
  * Author: Jakob Wenzel, Xilinx Research Labs.
@@ -87,6 +87,12 @@ protected void fill() throws IOException {
         int fillStart = (offset+available)&bufferAddressMask;
         int fillEnd = (offset-1) & bufferAddressMask;
 
+        // Make sure that we're overwriting the '0' that exists on initialization or the
+        // placeholder EOF character that was inserted on the last call to fill()
+        assert(buffer[fillStart] == 0);
+
+        // Set the last byte of the buffer to be 0 as a placeholder for EOF
+        // which will get overwritten on next fill()
         buffer[fillEnd] = 0;
 
         if (fillStart>fillEnd) {
@@ -146,16 +152,21 @@ public static String byteArrayToStringMulti(byte[] buffer, int start1, int lengt
      */
     protected String getUniqueToken(int startOffset, int endOffset, boolean isShortLived) {
         String token;
+        int length;
         if (endOffset >= startOffset) {
-            token = new String(buffer, startOffset, endOffset-startOffset, charset);
+            length = endOffset - startOffset;
+            token = new String(buffer, startOffset, length, charset);
         } else {
-            token = byteArrayToStringMulti(buffer, startOffset, buffer.length-startOffset, 0, endOffset);
+            int length1 = buffer.length - startOffset;
+            length = length1 + endOffset;
+            token = byteArrayToStringMulti(buffer, startOffset, length1, 0, endOffset);
+
         }
         if (!isShortLived) {
             token = uniquifier.uniquifyName(token);
         }
-        byteOffset+= token.length();
-        available-= token.length();
+        byteOffset += length;
+        available -= length;
         if (available<0) {
             throw new EDIFParseException("Token probably too long or failed to fetch data in time: "+ token +" at "+byteOffset);
         }
@@ -192,9 +203,10 @@ private String getQuotedToken(boolean isShortLived) throws IOException {
         return token;
     }
 
-    private IOException tokenTooLong(int bufferOffset) {
+    private IOException tokenTooLong(int startOffset) {
         final long byteOffsetAtStart = this.byteOffset;
-        final String failingToken = getUniqueToken(bufferOffset, bufferOffset + 150, true);
+        final int endOffset = bufferAddressMask & (startOffset + 150);
+        final String failingToken = getUniqueToken(startOffset, endOffset, true);
         throw new TokenTooLongException("ERROR: String buffer overflow on byte offset " +
                 byteOffsetAtStart + " parsing token starting with "+ failingToken +"...\n\t Please revisit why this EDIF token "
                 + "is so long or increase the buffer in " + this.getClass().getCanonicalName());

diff --git a/test/RapidWrightDCP b/test/RapidWrightDCP
diff --git a/test/src/com/xilinx/rapidwright/edif/TestEDIFParser.java b/test/src/com/xilinx/rapidwright/edif/TestEDIFParser.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2022, Xilinx, Inc.
- * Copyright (c) 2022, Advanced Micro Devices, Inc.
+ * Copyright (c) 2022, 2024, Advanced Micro Devices, Inc.
  * All rights reserved.
  *
  * Author: Jakob Wenzel, Xilinx Research Labs.
@@ -69,10 +69,10 @@ public class TestEDIFParser {
      * List of byte offsets that cause interesting behaviour if we start parsing there
      */
     private static final List<ParseStart> interestingOffsets = Arrays.asList(
-      new ParseStart("Mismatch in First Evil Cell", 517L, false),
-      new ParseStart("Mismatch in Second Evil Cell", 849L, false),
-      new ParseStart("Totally Misaligned", 1045L, false),
-      new ParseStart("After Evil", 1111L, true),
+      new ParseStart("Mismatch in First Evil Cell", 577L, false),
+      new ParseStart("Mismatch in Second Evil Cell", 919L, false),
+      new ParseStart("Totally Misaligned", 1115L, false),
+      new ParseStart("After Evil", 1181L, true),
       new ParseStart("At EOF", FILE_SIZE-2, false)
     );
 
@@ -85,10 +85,12 @@ public class TestEDIFParser {
     @ParameterizedTest(name="{0}")
     @MethodSource("testParallelArgs")
     public void testParallel(String ignoredDescription, List<ParseStart> offsets, int expectedSuccessfulThreads) throws IOException {
+        EDIFNetlist netlist;
         try (ParallelEDIFParserTestSpecificOffsets parser = new ParallelEDIFParserTestSpecificOffsets(input, 128, offsets)) {
-            parser.parseEDIFNetlist(new CodePerfTracker("parse edif"));
+            netlist = parser.parseEDIFNetlist(new CodePerfTracker("parse edif"));
             Assertions.assertEquals(expectedSuccessfulThreads, parser.getSuccessfulThreads());
         }
+        Assertions.assertTrue(netlist.getComments().contains("Here's some Unicode: àâæçéèêëœîïôùûÜüÿ"));
     }
 
     /**
@@ -125,10 +127,11 @@ public static Stream<Arguments> testParallelArgs() {
 
     @Test
     public void loadEDIFSingleThreaded() throws IOException {
+        EDIFNetlist netlist;
         try (EDIFParser parser = new EDIFParser(input)) {
-            parser.parseEDIFNetlist();
+            netlist = parser.parseEDIFNetlist();
         }
-
+        Assertions.assertTrue(netlist.getComments().contains("Here's some Unicode: àâæçéèêëœîïôùûÜüÿ"));
     }
 
     @ParameterizedTest