Merge branch 'master' of git://github.com/UpstandingHackers/hammer into fix-h_seq_index_path

SConstruct

@@ -14,7 +14,11 @@ tools = ['default', 'scanreplace']
 if 'dotnet' in ARGUMENTS.get('bindings', []):
 	tools.append('csharp/mono')
 
-env = Environment(ENV = {'PATH' : os.environ['PATH'], 'PKG_CONFIG_PATH' : os.environ['PKG_CONFIG_PATH']},
+envvars = {'PATH' : os.environ['PATH']}
+if 'PKG_CONFIG_PATH' in os.environ:
+    envvars['PKG_CONFIG_PATH'] = os.environ['PKG_CONFIG_PATH']
+
+env = Environment(ENV = envvars,
                   variables = vars,
                   tools=tools,
                   toolpath=['tools'])

src/SConscript

@@ -69,7 +69,8 @@ ctests = ['t_benchmark.c',
           't_bitwriter.c',
           't_parser.c',
           't_grammar.c',
-          't_misc.c']
+          't_misc.c',
+	  't_regression.c']
 
 libhammer_shared = env.SharedLibrary('hammer', parsers + backends + misc_hammer_parts)
 libhammer_static = env.StaticLibrary('hammer', parsers + backends + misc_hammer_parts)

src/allocator.h

@@ -33,11 +33,22 @@ typedef struct HAllocator_ {
 typedef struct HArena_ HArena ; // hidden implementation
 
 HArena *h_new_arena(HAllocator* allocator, size_t block_size); // pass 0 for default...
-#ifndef SWIG
-void* h_arena_malloc(HArena *arena, size_t count) __attribute__(( malloc, alloc_size(2) ));
+
+#if defined __llvm__
+# if __has_attribute(malloc)
+#   define ATTR_MALLOC(n) __attribute__((malloc))
+# else
+#   define ATTR_MALLOC(n)
+# endif
+#elif defined SWIG
+# define ATTR_MALLOC(n)
+#elif defined __GNUC__
+# define ATTR_MALLOC(n) __attribute__((malloc, alloc_size(2)))
 #else
-void* h_arena_malloc(HArena *arena, size_t count);
+# define ATTR_MALLOC(n)
 #endif
+
+void* h_arena_malloc(HArena *arena, size_t count) ATTR_MALLOC(2);
 void h_arena_free(HArena *arena, void* ptr); // For future expansion, with alternate memory managers.
 void h_delete_arena(HArena *arena);
 

src/backends/packrat.c

@@ -33,11 +33,13 @@ static inline HParseResult* perform_lowlevel_parse(HParseState *state, const HPa
     if (tmp_res) {
       tmp_res->arena = state->arena;
       if (!state->input_stream.overrun) {
-	tmp_res->bit_length = ((state->input_stream.index - bak.index) << 3);
-	if (state->input_stream.endianness & BIT_BIG_ENDIAN)
-	  tmp_res->bit_length += state->input_stream.bit_offset - bak.bit_offset;
-	else
-	  tmp_res->bit_length += bak.bit_offset - state->input_stream.bit_offset;
+	size_t bit_length = h_input_stream_pos(&state->input_stream) - h_input_stream_pos(&bak);
+	if (tmp_res->bit_length == 0) { // Don't modify if forwarding.
+	  tmp_res->bit_length = bit_length;
+	}
+	if (tmp_res->ast && tmp_res->ast->bit_length != 0) {
+	  ((HParsedToken*)(tmp_res->ast))->bit_length = bit_length;
+	}
       } else
 	tmp_res->bit_length = 0;
     }

src/bitreader.c

@@ -39,10 +39,7 @@ int64_t h_read_bits(HInputStream* state, int count, char signed_p) {
   if (bits_left <= 64) { // Large enough to handle any valid count, but small enough that overflow isn't a problem.
     // not in danger of overflowing, so add in bits
     // add in number of bits...
-    if (state->endianness & BIT_BIG_ENDIAN)
-      bits_left = (bits_left << 3) - 8 + state->bit_offset;
-    else
-      bits_left = (bits_left << 3) - state->bit_offset;
+    bits_left = (bits_left << 3) - state->bit_offset - state->margin;
     if (bits_left < count) {
       if (state->endianness & BYTE_BIG_ENDIAN)
 	final_shift = count - bits_left;
@@ -54,7 +51,7 @@ int64_t h_read_bits(HInputStream* state, int count, char signed_p) {
       final_shift = 0;
   }
   
-  if ((state->bit_offset & 0x7) == 0 && (count & 0x7) == 0) {
+  if ((state->bit_offset & 0x7) == 0 && (count & 0x7) == 0 && (state->margin == 0)) {
     // fast path
     if (state->endianness & BYTE_BIG_ENDIAN) {
       while (count > 0) {
@@ -73,22 +70,24 @@ int64_t h_read_bits(HInputStream* state, int count, char signed_p) {
       int segment, segment_len;
       // Read a segment...
       if (state->endianness & BIT_BIG_ENDIAN) {
-	if (count >= state->bit_offset) {
-	  segment_len = state->bit_offset;
-	  state->bit_offset = 8;
-	  segment = state->input[state->index] & ((1 << segment_len) - 1);
-	  state->index++;
-	} else {
-	  segment_len = count;
-	  state->bit_offset -= count;
-	  segment = (state->input[state->index] >> state->bit_offset) & ((1 << segment_len) - 1);
-	}
-      } else { // BIT_LITTLE_ENDIAN
-	if (count + state->bit_offset >= 8) {
-	  segment_len = 8 - state->bit_offset;
-	  segment = (state->input[state->index] >> state->bit_offset);
+	if (count + state->bit_offset + state->margin >= 8) {
+	  segment_len = 8 - state->bit_offset - state->margin;
+	  segment = (state->input[state->index] >> state->margin) & ((1 << segment_len) - 1);
 	  state->index++;
 	  state->bit_offset = 0;
+	  state->margin = 0;
+	} else {
+	  segment_len = count;
+	  state->bit_offset += count;
+	  segment = (state->input[state->index] >> (8 - state->bit_offset)) & ((1 << segment_len) - 1);
+	}
+      } else { // BIT_LITTLE_ENDIAN
+	if (count + state->bit_offset + state->margin >= 8) {
+	  segment_len = 8 - state->bit_offset - state->margin;
+	  segment = (state->input[state->index] >> state->bit_offset) & ((1 << segment_len) - 1);
+	  state->index++;
+	  state->bit_offset = 0;
+	  state->margin = 0;
 	} else {
 	  segment_len = count;
 	  segment = (state->input[state->index] >> state->bit_offset) & ((1 << segment_len) - 1);

src/glue.h

@@ -11,7 +11,8 @@
 //
 // A few standard semantic actions are defined below. The H_ACT_APPLY macro
 // allows semantic actions to be defined by "partial application" of
-// a generic action to fixed paramters.
+// a generic action to fixed paramters. H_VALIDATE_APPLY is similar for
+// h_atter_bool.
 //
 // The definition of more complex semantic actions will usually consist of
 // extracting data from the given parse tree and constructing a token of custom
@@ -66,13 +67,13 @@
     h_attr_bool(h_action(def, act_ ## rule, NULL), validate_ ## rule, NULL)
 #define H_AVRULE(rule, def) HParser *rule = \
     h_action(h_attr_bool(def, validate_ ## rule, NULL), act_ ## rule, NULL)
-#define H_ADRULE(rule, def, data) HParser *rule =	\
+#define H_ADRULE(rule, def, data) HParser *rule =       \
     h_action(def, act_ ## rule, data)
-#define H_VDRULE(rule, def, data) HParser *rule =	\
+#define H_VDRULE(rule, def, data) HParser *rule =       \
     h_attr_bool(def, validate_ ## rule, data)
-#define H_VADRULE(rule, def, data) HParser *rule =		\
+#define H_VADRULE(rule, def, data) HParser *rule =              \
     h_attr_bool(h_action(def, act_ ## rule, data), validate_ ## rule, data)
-#define H_AVDRULE(rule, def, data) HParser *rule =		\
+#define H_AVDRULE(rule, def, data) HParser *rule =              \
     h_action(h_attr_bool(def, validate_ ## rule, data), act_ ## rule, data)
 
 
@@ -109,8 +110,14 @@ HParsedToken *h_act_ignore(const HParseResult *p, void* user_data);
 // Define 'myaction' as a specialization of 'paction' by supplying the leading
 // parameters.
 #define H_ACT_APPLY(myaction, paction, ...) \
-  HParsedToken *myaction(const HParseResult *p, void* user_data) {	\
-    return paction(__VA_ARGS__, p, user_data);				\
+  HParsedToken *myaction(const HParseResult *p, void* user_data) {      \
+    return paction(__VA_ARGS__, p, user_data);                          \
+  }
+
+// Similar, but for validations.
+#define H_VALIDATE_APPLY(myvalidation, pvalidation, ...)  \
+  bool myvalidation(HParseResult* p, void* user_data) {   \
+    return pvalidation(__VA_ARGS__, p, user_data);        \
   }
 
 

src/hammer.c

@@ -52,7 +52,7 @@ HParseResult* h_parse__m(HAllocator* mm__, const HParser* parser, const uint8_t*
   // Set up a parse state...
   HInputStream input_stream = {
     .index = 0,
-    .bit_offset = 8,
+    .bit_offset = 0,
     .overrun = 0,
     .endianness = BIT_BIG_ENDIAN | BYTE_BIG_ENDIAN,
     .length = length,

src/hammer.h

@@ -99,6 +99,7 @@ typedef struct HParsedToken_ {
   HTokenData token_data;
 #endif
   size_t index;
+  size_t bit_length;
   char bit_offset;
 } HParsedToken;
 

src/internal.h

@@ -70,6 +70,8 @@ typedef struct HInputStream_ {
   size_t index;
   size_t length;
   char bit_offset;
+  char margin; // The number of bits on the end that is being read
+	       // towards that should be ignored.
   char endianness;
   char overrun;
 } HInputStream;
@@ -295,6 +297,9 @@ extern HParserBackendVTable h__glr_backend_vtable;
 // TODO(thequux): Set symbol visibility for these functions so that they aren't exported.
 
 int64_t h_read_bits(HInputStream* state, int count, char signed_p);
+static inline size_t h_input_stream_pos(HInputStream* state) {
+  return state->index * 8 + state->bit_offset + state->margin;
+}
 // need to decide if we want to make this public. 
 HParseResult* h_do_parse(const HParser* parser, HParseState *state);
 void put_cached(HParseState *ps, const HParser *p, HParseResult *cached);

src/parsers/endianness.c

@@ -11,19 +11,9 @@ static void switch_bit_order(HInputStream *input)
 {
     assert(input->bit_offset <= 8);
 
-    if((input->bit_offset % 8) != 0) {
-        // switching bit order in the middle of a byte
-        // we leave bit_offset untouched. this means that something like
-        //     le(bits(5)),le(bits(3))
-        // is equivalent to
-        //     le(bits(5),bits(3)) .
-        // on the other hand,
-        //     le(bits(5)),be(bits(5))
-        // will read the same 5 bits twice and discard the top 3.
-    } else {
-        // flip offset (0 <-> 8)
-        input->bit_offset = 8 - input->bit_offset;
-    }
+    char tmp = input->bit_offset;
+    input->bit_offset = input->margin;
+    input->margin = tmp;
 }
 
 static HParseResult *parse_endianness(void *env, HParseState *state)

src/parsers/parser_internal.h

@@ -18,6 +18,7 @@ static inline HParseResult* make_result(HArena *arena, HParsedToken *tok) {
   HParseResult *ret = h_arena_malloc(arena, sizeof(HParseResult));
   ret->ast = tok;
   ret->arena = arena;
+  ret->bit_length = 0; // This way it gets overridden in h_do_parse
   return ret;
 }
 

src/t_bitreader.c

@@ -4,14 +4,14 @@
 #include "internal.h"
 #include "test_suite.h"
 
-#define MK_INPUT_STREAM(buf,len,endianness_)   \
+#define MK_INPUT_STREAM(buf,len,endianness_)  \
   {					      \
-    .input = (uint8_t*)buf,					\
-      .length = len,						\
-      .index = 0,						\
-      .bit_offset = (((endianness_) & BIT_BIG_ENDIAN) ? 8 : 0),	\
-      .endianness = endianness_					\
-      }
+      .input = (uint8_t*)buf,		      \
+      .length = len,			      \
+      .index = 0,			      \
+      .bit_offset = 0,			      \
+      .endianness = endianness_		      \
+  }
 
 
 static void test_bitreader_ints(void) {
@@ -56,7 +56,6 @@ static void test_offset_largebits_le(void) {
   g_check_cmp_int32(h_read_bits(&is, 11, false), ==, 0x2D3);
 }
 
-
 void register_bitreader_tests(void)  {
   g_test_add_func("/core/bitreader/be", test_bitreader_be);
   g_test_add_func("/core/bitreader/le", test_bitreader_le);

src/t_bitwriter.c

@@ -24,7 +24,7 @@ void run_bitwriter_test(bitwriter_test_elem data[], char flags) {
     .input = buf,
     .index = 0,
     .length = len,
-    .bit_offset = (flags & BIT_BIG_ENDIAN) ? 8 : 0,
+    .bit_offset = 0,
     .endianness = flags,
     .overrun = 0
   };

src/t_regression.c

@@ -0,0 +1,38 @@
+#include <glib.h>
+#include <stdint.h>
+#include "glue.h"
+#include "hammer.h"
+#include "test_suite.h"
+
+static void test_bug118(void) {
+  // https://github.com/UpstandingHackers/hammer/issues/118
+  // Adapted from https://gist.github.com/mrdomino/c6bc91a7cb3b9817edb5
+
+  HParseResult* p;
+  const uint8_t *input = (uint8_t*)"\x69\x5A\x6A\x7A\x8A\x9A";
+ 
+#define MY_ENDIAN (BIT_BIG_ENDIAN | BYTE_LITTLE_ENDIAN)
+    H_RULE(nibble, h_with_endianness(MY_ENDIAN, h_bits(4, false)));
+    H_RULE(sample, h_with_endianness(MY_ENDIAN, h_bits(10, false)));
+#undef MY_ENDIAN
+ 
+    H_RULE(samples, h_sequence(h_repeat_n(sample, 3), h_ignore(h_bits(2, false)), NULL));
+ 
+    H_RULE(header_ok, h_sequence(nibble, nibble, NULL));
+    H_RULE(header_weird, h_sequence(nibble, nibble, nibble, NULL));
+ 
+    H_RULE(parser_ok, h_sequence(header_ok, samples, NULL));
+    H_RULE(parser_weird, h_sequence(header_weird, samples, NULL));
+ 
+ 
+    p = h_parse(parser_weird, input, 6);
+    g_check_cmp_int32(p->bit_length, ==, 44);
+    h_parse_result_free(p);
+    p = h_parse(parser_ok, input, 6);
+    g_check_cmp_int32(p->bit_length, ==, 40);
+    h_parse_result_free(p);
+}
+
+void register_regression_tests(void) {
+  g_test_add_func("/core/regression/bug118", test_bug118);
+}

src/test_suite.c

@@ -25,6 +25,7 @@ extern void register_parser_tests();
 extern void register_grammar_tests();
 extern void register_misc_tests();
 extern void register_benchmark_tests();
+extern void register_regression_tests();
 
 int main(int argc, char** argv) {
   g_test_init(&argc, &argv, NULL);
@@ -35,6 +36,7 @@ int main(int argc, char** argv) {
   register_parser_tests();
   register_grammar_tests();
   register_misc_tests();
+  register_regression_tests();
   if (g_test_slow() || g_test_perf())
     register_benchmark_tests();