This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
BZ 6701 - Improve error messages (patch)
- From: Rajan Arora <rarora at redhat dot com>
- To: systemtap at sources dot redhat dot com
- Date: Tue, 30 Sep 2008 17:46:10 -0400
- Subject: BZ 6701 - Improve error messages (patch)
- Organization: Red Hat
- Reply-to: rarora at redhat dot com
Hi:
This is a proposed patch for the Bugzilla bug 6701
(http://sources.redhat.com/bugzilla/show_bug.cgi?id=6701) to improve
error/warning messages with source context. After application of the
patch, error/warning messages should be followed by a handy ``source: "
line and an ascii `^' character in the following line pointing at the
exact column. Hence, displaying the line and column where the error was
encountered and in case of some parse errors, the last source line
visited. Improvements have been made by adding a ``print_error_source''
method to the systemtap_session struct which does the actual work of
displaying the error causing line and col once it has been encountered.
New error messages look something like this:
# stap -ve 'probe begin { log("hello world") exit () }'
Pass 1: parsed user script and 45 library script(s) in 150usr/10sys/172real ms.
semantic error: unresolved arity-0 function: identifier 'exxit' at <input>:1:34
source: probe begin { log("hello world") exxit () }
^
Changes have been made to testsuite/systemtap.base/warnings.exp so it
recognizes the ``source: " lines and those with the `^' character. A
test script has also been added to the testuite/parseko directory. The
following patch file may be applied to have a feel of how it looks and
any suggestions/comments are welcome. Since, this is my first patch, I
hope I can improve this with some feedback from the group.
best,
-Rajan
diff --git a/ChangeLog b/ChangeLog
index ea1d51d..e84aac6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2008-09-30 Rajan Arora <rarora@redhat.com>
+
+ * elaborate.cxx (systemtap_session::print_error_source): New.
+ (systemtap_session::print_error): Call it.
+ (systemtap_session::print_warning): Likewise.
+ * parse.cxx (parser::print_error): Likewise.
+ * session.h (struct systemtap_session::print_error_source):
+ Declare it.
+ * parse.cxx (lexer::get_input_contents): New.
+ (parser::parser): Call it.
+ * parse.h (class lexer::get_input_contents): Declare it.
+ * staptree.h (struct stapfile): New member file_contents.
+
2008-09-30 Mark Wielaard <mjw@redhat.com>
* tapsets.cxx (literal_stmt_for_local): Check if alternatives can be
diff --git a/elaborate.cxx b/elaborate.cxx
index 94bfd1c..9ead814 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1425,6 +1425,7 @@ void
systemtap_session::print_error (const semantic_error& e)
{
string message_str[2];
+ stringstream errormsg;
// NB: we don't print error messages during listing mode.
if (listing_mode) return;
@@ -1461,6 +1462,20 @@ systemtap_session::print_error (const semantic_error& e)
{
seen_errors.insert (message_str[1]);
cerr << message_str[0];
+
+ //Do file_contents exist? If not, error is in tapset!
+ if (e.tok1 && user_file->file_contents.size() != 0)
+ {
+ errormsg.str ("");
+ print_error_source (errormsg, user_file->file_contents, e.tok1);
+ cerr << errormsg.str();
+ }
+ if (e.tok2 && user_file->file_contents.size() != 0)
+ {
+ errormsg.str ("");
+ print_error_source (errormsg, user_file->file_contents, e.tok2);
+ cerr << errormsg.str();
+ }
}
if (e.chain)
@@ -1468,15 +1483,41 @@ systemtap_session::print_error (const semantic_error& e)
}
void
+systemtap_session::print_error_source (std::stringstream& message,
+ std::string& file_contents, const token* tok)
+{
+ unsigned i = 0;
+ unsigned line = tok->location.line;
+ unsigned col = tok->location.column;
+ size_t start_pos = 0, end_pos = 0;
+ //Navigate to the appropriate line
+ while (i != line && end_pos != std::string::npos)
+ {
+ start_pos = end_pos;
+ end_pos = file_contents.find ('\n', start_pos) + 1;
+ i++;
+ }
+ message << "\tsource: " << file_contents.substr (start_pos, end_pos-start_pos-1) << endl;
+ message << "\t ";
+ message.fill (' ');
+ message.width (col);
+ message << "^" << endl;
+}
+
+void
systemtap_session::print_warning (const string& message_str, const token* tok)
{
// Duplicate elimination
+ stringstream message;
if (seen_warnings.find (message_str) == seen_warnings.end())
{
seen_warnings.insert (message_str);
clog << "WARNING: " << message_str;
if (tok) { clog << ": "; print_token (clog, tok); }
clog << endl;
+ message.str ("");
+ print_error_source (message, user_file->file_contents, tok);
+ clog << message.str();
}
}
diff --git a/parse.cxx b/parse.cxx
index 1c1772f..7848250 100644
--- a/parse.cxx
+++ b/parse.cxx
@@ -124,17 +124,28 @@ operator << (ostream& o, const token& t)
void
parser::print_error (const parse_error &pe)
{
+ stringstream error_source;
+ string input_contents;
+ input.get_input_contents (input_contents);
cerr << "parse error: " << pe.what () << endl;
if (pe.tok)
{
cerr << "\tat: " << *pe.tok << endl;
+ error_source.str ("");
+ session.print_error_source (error_source, input_contents, pe.tok);
+ cerr << " " << error_source.str ();
}
else
{
const token* t = last_t;
if (t)
- cerr << "\tsaw: " << *t << endl;
+ {
+ cerr << "\tsaw: " << *t << endl;
+ error_source.str ("");
+ session.print_error_source (error_source, input_contents, t);
+ cerr << error_source.str ();
+ }
else
cerr << "\tsaw: " << input_name << " EOF" << endl;
}
@@ -588,6 +599,13 @@ lexer::lexer (istream& i, const string& in, systemtap_session& s):
input_contents.push_back(c);
}
+void
+lexer::get_input_contents (std::string& file_contents)
+{
+ unsigned i;
+ for (i=0; i<input_contents.size(); i++)
+ file_contents += input_contents[i];
+}
int
lexer::input_peek (unsigned n)
@@ -1021,6 +1039,9 @@ parser::parse ()
delete f;
return 0;
}
+ //Assuming tapsets are error-free, only user's script contents are fetched
+ if (!strstr (input_name.c_str(),"tapset/"))
+ input.get_input_contents (f->file_contents);
return f;
}
diff --git a/parse.h b/parse.h
index cf31f4f..84ba572 100644
--- a/parse.h
+++ b/parse.h
@@ -71,6 +71,7 @@ class lexer
public:
token* scan (bool wildcard=false);
lexer (std::istream&, const std::string&, systemtap_session&);
+ void get_input_contents (std::string&);
private:
int input_get ();
diff --git a/session.h b/session.h
index b8bd971..9f272b3 100644
--- a/session.h
+++ b/session.h
@@ -179,6 +179,7 @@ struct systemtap_session
const token* last_token;
void print_token (std::ostream& o, const token* tok);
void print_error (const semantic_error& e);
+ void print_error_source (std::stringstream&, std::string&, const token* tok);
void print_warning (const std::string& w, const token* tok = 0);
// reNB: new POD members likely need to be explicitly cleared in the ctor.
diff --git a/staptree.h b/staptree.h
index 770a731..ac229c7 100644
--- a/staptree.h
+++ b/staptree.h
@@ -574,6 +574,7 @@ struct stapfile
std::vector<functiondecl*> functions;
std::vector<vardecl*> globals;
std::vector<embeddedcode*> embeds;
+ std::string file_contents;
bool privileged;
stapfile (): privileged (false) {}
void print (std::ostream& o) const;
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index f3fad07..f3b8ca4 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2008-09-26 Rajan Arora <rarora@redhat.com>
+
+ * systemtap.base/warnings.exp: Allow for source: lines.
+ * parseko/source_context.stp: New file.
+
2008-09-26 Frank Ch. Eigler <fche@elastic.org>
PR 6916.
diff --git a/testsuite/parseko/source_context.stp b/testsuite/parseko/source_context.stp
new file mode 100755
index 0000000..e718d9e
--- /dev/null
+++ b/testsuite/parseko/source_context.stp
@@ -0,0 +1,11 @@
+global count_test
+probe timer.ms(123)
+{
+printf("%d loops have executed\n",count_test)
+count_test ++
+iffff(count_test == 5)
+{
+priiintf("Done execution 5 times and about to exit. . .\n")
+ eeexit ()
+}
+}
diff --git a/testsuite/systemtap.base/warnings.exp b/testsuite/systemtap.base/warnings.exp
index a90860d..f6bfa34 100644
--- a/testsuite/systemtap.base/warnings.exp
+++ b/testsuite/systemtap.base/warnings.exp
@@ -6,6 +6,8 @@ expect {
-timeout 30
-re {^WARNING:[^\r\n]*\r\n} { incr ok; exp_continue }
-re {^[^\r\n]*.ko\r\n} { incr ok; exp_continue }
+ -re {^source:[^\r\n]*\r\n} {exp_continue}
+ -re {^[^\r\n]*\^[^\r\n]*\r\n} {exp_continue}
timeout { fail "$test (timeout)" }
eof { }
}