[PATCH, MELT] correct meltgc_read_from_val without location

[PATCH, MELT] correct meltgc_read_from_val without location

[Pierre Vittet]

[-- Attachment #1: Type: text/plain, Size: 463 bytes --]

Hello,

The function meltgc_read_from_val (in melt-runtime.c) takes two 
arguments, a string value and a second one which is a location.
In the comments, it is written that we can pass a NULL pointer if we 
have no location (it is a direct string). However, this conduct MELT to 
crash because it doesn't handle correctly the absence of file.

This patch correct this, if there is no file, it create a "virtual" one 
which is named "stringBuffer".

Pierre Vittet

[-- Attachment #2: correct_read_from_val_without_location-175348.diff --]
[-- Type: text/plain, Size: 2264 bytes --]

Index: gcc/melt-runtime.c
===================================================================
--- gcc/melt-runtime.c	(revision 175348)
+++ gcc/melt-runtime.c	(working copy)
@@ -6326,7 +6326,7 @@ melt_linemap_compute_current_location (struct read
 {
   int colnum = 1;
   int cix = 0;
-  if (!rd || !rd->rcurlin) 
+  if (!rd || !rd->rcurlin || !rd->rpfilnam)
     return;
   for (cix=0; cix<rd->rcol; cix++) {
     char c = rd->rcurlin[cix];
@@ -6702,13 +6702,22 @@ static melt_ptr_t
 makesexpr (struct reading_st *rd, int lineno, melt_ptr_t contents_p,
 	   location_t loc, bool ismacrostring)
 {
-  MELT_ENTERFRAME (4, NULL);
+  MELT_ENTERFRAME (5, NULL);
 #define sexprv  meltfram__.mcfr_varptr[0]
 #define contsv   meltfram__.mcfr_varptr[1]
 #define locmixv meltfram__.mcfr_varptr[2]
 #define sexpclassv meltfram__.mcfr_varptr[3]
+#define locnamv meltfram__.mcfr_varptr[4]
   contsv = contents_p;
   gcc_assert (melt_magic_discr ((melt_ptr_t) contsv) == MELTOBMAG_LIST);
+  /* If there is no filename associated, we create a false one, named
+    "stringBuffer".  */
+  if(rd->rpfilnam == NULL)
+    {
+      locnamv = meltgc_new_string ((meltobject_ptr_t) MELT_PREDEF(DISCR_STRING),
+                                   "stringBuffer");
+      rd->rpfilnam = (melt_ptr_t *) &locnamv;
+    }
   if (loc == 0)
     locmixv = meltgc_new_mixint ((meltobject_ptr_t) MELT_PREDEF (DISCR_MIXED_INTEGER),
 				    *rd->rpfilnam, (long) lineno);
@@ -6728,6 +6737,7 @@ makesexpr (struct reading_st *rd, int lineno, melt
   meltgc_touch (sexprv);
   MELT_EXITFRAME ();
   return (melt_ptr_t) sexprv;
+#undef locnamv
 #undef sexprv
 #undef contsv
 #undef locmixv
@@ -8414,6 +8424,7 @@ meltgc_read_from_val (melt_ptr_t strv_p, melt_ptr_
   strv = strv_p;
   locnamv = locnam_p;
   rbuf = 0;
+  seqv = meltgc_new_list ((meltobject_ptr_t) MELT_PREDEF (DISCR_LIST));
   strmagic = melt_magic_discr ((melt_ptr_t) strv);
   switch (strmagic)
     {
@@ -8442,7 +8453,10 @@ meltgc_read_from_val (melt_ptr_t strv_p, melt_ptr_
   rds.rlineno = 0;
   rds.rcurlin = rbuf;
   rd = &rds;
-  rds.rpfilnam = (melt_ptr_t *) & locnamv;
+  if(locnamv == NULL)
+    rds.rpfilnam = NULL;
+  else
+    rds.rpfilnam = (melt_ptr_t *) & locnamv;
   while (rdcurc ())
     {
       bool got = FALSE;

[-- Attachment #3: correct_read_from_val_without_location-175348.ChangeLog --]
[-- Type: text/plain, Size: 209 bytes --]

2011-06-24  Pierre Vittet  <piervit@pvittet.com>

	* melt-runtime.c (melt_linemap_compute_current_location, makesexpr,
	meltgc_read_from_val): Handle the case of reading a string sexp without
	given location.

Re: [PATCH, MELT] correct meltgc_read_from_val without location

[Pierre]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

Hello,

here is an improvment to 
http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01888.html.

The function meltgc_read_from_val (in melt-runtime.c) takes two 
arguments, a string value and a second one which is a location. In the 
comments, it is written that we can pass a NULL pointer if we have no 
location (it is a direct string). However, this conduct MELT to crash 
because it doesn't handle correctly the absence of file.

In the first patch, I modified makesexpr to create a location with a 
'virtual' file. This works however, as this function is used recursively 
this is not very elegant.

This patch is more elegant, it adds a boolean field to the struct 
reading_st to declare if there is no file location.
meltgc_read_* functions are modified to test if there is a given 
location, and if no, it create a location with a virtual file and set 
the boolean to false, conducting to avoid crash.

I have been able to build MELT with this pass and test it without problem.

On 24/06/2011 18:13, Pierre Vittet wrote:
> Hello,
>
> The function meltgc_read_from_val (in melt-runtime.c) takes two
> arguments, a string value and a second one which is a location.
> In the comments, it is written that we can pass a NULL pointer if we
> have no location (it is a direct string). However, this conduct MELT to
> crash because it doesn't handle correctly the absence of file.
>
> This patch correct this, if there is no file, it create a "virtual" one
> which is named "stringBuffer".
>
> Pierre Vittet
>
>
> correct_read_from_val_without_location-175348.diff
>
>


[-- Attachment #2: correct_read_from_val_without_location-175348.diff --]
[-- Type: text/plain, Size: 2585 bytes --]

Index: gcc/melt-runtime.c
===================================================================
--- gcc/melt-runtime.c	(revision 175348)
+++ gcc/melt-runtime.c	(working copy)
@@ -6292,6 +6292,7 @@ struct reading_st
   int rcol;			/* current column */
   source_location rsrcloc;	/* current source location */
   melt_ptr_t *rpfilnam;	/* pointer to location of file name string */
+  bool has_file_location;	/* precise if the string comes from a file */
 };
 
 #define MELT_READ_TABULATION_FACTOR 8
@@ -6326,7 +6327,7 @@ melt_linemap_compute_current_location (struct read
 {
   int colnum = 1;
   int cix = 0;
-  if (!rd || !rd->rcurlin) 
+  if (!rd || !rd->rcurlin || !rd->has_file_location)
     return;
   for (cix=0; cix<rd->rcol; cix++) {
     char c = rd->rcurlin[cix];
@@ -8314,6 +8315,7 @@ meltgc_read_file (const char *filnam, const char *
   rd = &rds;
   locnamv = meltgc_new_stringdup ((meltobject_ptr_t) MELT_PREDEF (DISCR_STRING), locnam);
   rds.rpfilnam = (melt_ptr_t *) & locnamv;
+  rds.has_file_location = true;
   seqv = meltgc_new_list ((meltobject_ptr_t) MELT_PREDEF (DISCR_LIST));
   while (!rdeof ())
     {
@@ -8371,7 +8373,16 @@ meltgc_read_from_rawstring (const char *rawstr, co
   rds.rsrcloc = loch;
   rd = &rds;
   if (locnam)
-    locnamv = meltgc_new_stringdup ((meltobject_ptr_t) MELT_PREDEF (DISCR_STRING), locnam);
+    {
+      rds.has_file_location = true;
+      locnamv = meltgc_new_stringdup ((meltobject_ptr_t) MELT_PREDEF (DISCR_STRING), locnam);
+    }
+  else
+    {
+      rds.has_file_location = false;
+      locnamv = meltgc_new_string ((meltobject_ptr_t) MELT_PREDEF(DISCR_STRING),
+				   "stringBuffer");
+    }
   seqv = meltgc_new_list ((meltobject_ptr_t) MELT_PREDEF (DISCR_LIST));
   rds.rpfilnam = (melt_ptr_t *) & locnamv;
   while (rdcurc ())
@@ -8415,6 +8426,7 @@ meltgc_read_from_val (melt_ptr_t strv_p, melt_ptr_
   locnamv = locnam_p;
   rbuf = 0;
   strmagic = melt_magic_discr ((melt_ptr_t) strv);
+  seqv = meltgc_new_list ((meltobject_ptr_t) MELT_PREDEF (DISCR_LIST));
   switch (strmagic)
     {
     case MELTOBMAG_STRING:
@@ -8441,7 +8453,14 @@ meltgc_read_from_val (melt_ptr_t strv_p, melt_ptr_
   rds.rpath = 0;
   rds.rlineno = 0;
   rds.rcurlin = rbuf;
+  rds.has_file_location = true;
   rd = &rds;
+  if (locnamv == NULL){
+    rds.has_file_location = false;
+    locnamv = meltgc_new_string ((meltobject_ptr_t) MELT_PREDEF(DISCR_STRING),
+                                 "stringBuffer");
+    rd->rpfilnam = (melt_ptr_t *) &locnamv;
+  }
   rds.rpfilnam = (melt_ptr_t *) & locnamv;
   while (rdcurc ())
     {

[-- Attachment #3: correct_read_from_val_without_location-175348.ChangeLog --]
[-- Type: text/plain, Size: 398 bytes --]

2011-06-29  Pierre Vittet  <piervit@pvittet.com>

	* melt-runtime.c (struct reading_st): add a boolean has_file_location
	field.
	(melt_linemap_compute_current_location): return immediately if no file
	location.
	(meltgc_read_file, meltgc_read_from_rawstring, meltgc_read_from_val):
	set has_file_location accordingly
	(meltgc_read_from_val): create seqv list (it was used without being
	created).

Re: [PATCH, MELT] correct meltgc_read_from_val without location

[Basile Starynkevitch]

On Wed, 29 Jun 2011 20:32:08 +0200
Pierre <p.vittet@laposte.net> wrote:

> Hello,
> 
> here is an improvment to 
> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg01888.html.
> 

Thanks. I applied it with minor changes on the MELT branch.
{spelling mistakes & indentation mostly}.

Perhaps a future improvement might be to set the location (i.e. locnamv
in meltgc_read_from_val) to something which depends upon the string
rbuf. I have no idea if that would be useful.

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***