ACK: [PATCH 2/2] iasl: capture output from stderr to catch iasl fatal errors
IvanHu
ivan.hu at canonical.com
Wed Oct 23 09:12:00 UTC 2013
On 10/03/2013 06:36 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> ACPICA commit 7bbc5f141be1b15e232da253d331dda6de4d8b1c now
> makes iasl abort immediately on serious unrecoverable parse errors
> and emits an error message on stderr. We need to capture this and
> pass it up to the syntaxcheck test to report back that iasl has
> aborted early. The upside is that syntaxcheck now does not report
> tens of parser errors.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
> src/acpi/syntaxcheck/syntaxcheck.c | 31 ++++++--
> src/acpica/source/compiler/fwts_iasl_interface.c | 98 +++++++++++++++++-------
> src/acpica/source/compiler/fwts_iasl_interface.h | 2 +-
> src/lib/include/fwts_iasl.h | 5 +-
> src/lib/src/fwts_iasl.c | 18 +++--
> 5 files changed, 109 insertions(+), 45 deletions(-)
>
> diff --git a/src/acpi/syntaxcheck/syntaxcheck.c b/src/acpi/syntaxcheck/syntaxcheck.c
> index 9bd6711..749bd1b 100644
> --- a/src/acpi/syntaxcheck/syntaxcheck.c
> +++ b/src/acpi/syntaxcheck/syntaxcheck.c
> @@ -441,8 +441,7 @@ static int syntaxcheck_table(fwts_framework *fw, char *tablename, int which)
> int warnings = 0;
> int remarks = 0;
> fwts_acpi_table_info *table;
> - fwts_list* iasl_errors;
> - fwts_list* iasl_disassembly;
> + fwts_list *iasl_stdout, *iasl_stderr, *iasl_disassembly;
>
> if (fwts_acpi_find_table(fw, tablename, which, &table) != FWTS_OK) {
> fwts_aborted(fw, "Cannot load ACPI table %s.", tablename);
> @@ -453,7 +452,7 @@ static int syntaxcheck_table(fwts_framework *fw, char *tablename, int which)
> return FWTS_NO_TABLE; /* Table does not exist */
>
> if (fwts_iasl_reassemble(fw, table->data, table->length,
> - &iasl_disassembly, &iasl_errors) != FWTS_OK) {
> + &iasl_disassembly, &iasl_stdout, &iasl_stderr) != FWTS_OK) {
> fwts_aborted(fw, "Cannot re-assasemble with iasl.");
> return FWTS_ERROR;
> }
> @@ -462,9 +461,9 @@ static int syntaxcheck_table(fwts_framework *fw, char *tablename, int which)
> fwts_log_info(fw, "Checking ACPI table %s (#%d)", tablename, which);
> fwts_log_nl(fw);
>
> - if (iasl_errors) {
> + if (iasl_stdout) {
> /* Scan error text from assembly */
> - fwts_list_foreach(item, iasl_errors) {
> + fwts_list_foreach(item, iasl_stdout) {
> int num;
> char ch;
> char *line = fwts_text_list_text(item);
> @@ -554,8 +553,28 @@ static int syntaxcheck_table(fwts_framework *fw, char *tablename, int which)
> }
> }
> }
> +
> + /*
> + * Did iasl emit any errors that we need to check?
> + */
> + if (iasl_stderr) {
> + fwts_list_foreach(item, iasl_stderr) {
> + char *line = fwts_text_list_text(item);
> + if (strstr(line, "Compiler aborting due to parser-detected syntax error")) {
> + fwts_failed(fw, LOG_LEVEL_HIGH, "SyntaxCheckIASLCompilerAborted",
> + "Compilation aborted early due to a parser detected syntax error.");
> + fwts_advice(fw,
> + "Some subsequent errors may not be detected because the "
> + "compiler had to terminate prematurely. If the compiler did not "
> + "abort early then potentially correct code may parse incorrectly "
> + "producing some or many false positive errors.");
> + }
> + }
> + }
> +
> fwts_text_list_free(iasl_disassembly);
> - fwts_text_list_free(iasl_errors);
> + fwts_text_list_free(iasl_stdout);
> + fwts_text_list_free(iasl_stderr);
>
> if (errors + warnings + remarks > 0)
> fwts_log_info(fw, "Table %s (%d) reassembly: Found %d errors, %d warnings, %d remarks.",
> diff --git a/src/acpica/source/compiler/fwts_iasl_interface.c b/src/acpica/source/compiler/fwts_iasl_interface.c
> index 49278ec..607dec3 100644
> --- a/src/acpica/source/compiler/fwts_iasl_interface.c
> +++ b/src/acpica/source/compiler/fwts_iasl_interface.c
> @@ -19,6 +19,8 @@
>
> #define _DECLARE_GLOBALS
>
> +#include <unistd.h>
> +#include <stdbool.h>
> #include <sys/types.h>
> #include <sys/wait.h>
>
> @@ -27,6 +29,10 @@
> #include "aslcompiler.h"
> #include "acapps.h"
>
> +/*
> + * init_asl_core()
> + * initialize iasl
> + */
> static void init_asl_core(void)
> {
> int i;
> @@ -49,6 +55,10 @@ static void init_asl_core(void)
> UtExpandLineBuffers();
> }
>
> +/*
> + * fwts_iasl_disassemble_aml()
> + * invoke iasl to disassemble AML
> + */
> int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
> {
> pid_t pid;
> @@ -63,6 +73,8 @@ int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
> init_asl_core();
>
> /* Setup ACPICA disassembler globals */
> + Gbl_WarningLevel = ASL_WARNING3;
> + Gbl_IgnoreErrors = TRUE;
> Gbl_DisasmFlag = TRUE;
> Gbl_DoCompile = FALSE;
> Gbl_OutputFilenamePrefix = (char*)outputfile;
> @@ -82,38 +94,70 @@ int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile)
> return 0;
> }
>
> -int fwts_iasl_assemble_aml(const char *source, char **output)
> +/*
> + * fwts_iasl_read_output()
> + * consume output from iasl.
> + */
> +static int fwts_iasl_read_output(const int fd, char **data, size_t *len, bool *eof)
> {
> - int pipefds[2];
> - int status;
> - int ret = 0;
> - size_t len = 0;
> + char buffer[8192];
> ssize_t n;
> +
> + if (*eof)
> + return 0;
> +
> + while ((n = read(fd, buffer, sizeof(buffer))) > 0) {
> + if ((*data = realloc(*data, *len + n + 1)) == NULL)
> + return -1;
> + memcpy(*data + *len, buffer, n);
> + *len += n;
> + (*data)[*len] = '\0';
> + }
> + if (n <= 0)
> + *eof = true;
> + return 0;
> +}
> +
> +/*
> + * fwts_iasl_assemble_aml()
> + * invoke iasl and assemble some source, stdout into
> + * stdout_output, stderr into stderr_output
> + */
> +int fwts_iasl_assemble_aml(const char *source, char **stdout_output, char **stderr_output)
> +{
> + int stdout_fds[2], stderr_fds[2];
> + int status, ret = 0;
> + size_t stdout_len = 0, stderr_len = 0;
> pid_t pid;
> - char *data = NULL;
> - char buffer[8192];
> + bool stdout_eof = false, stderr_eof = false;
>
> - if (pipe(pipefds) < 0)
> + if (pipe(stdout_fds) < 0)
> + return -1;
> + if (pipe(stderr_fds) < 0)
> return -1;
>
> pid = fork();
> switch (pid) {
> case -1:
> - (void)close(pipefds[0]);
> - (void)close(pipefds[1]);
> + (void)close(stdout_fds[0]);
> + (void)close(stdout_fds[1]);
> + (void)close(stderr_fds[0]);
> + (void)close(stderr_fds[1]);
> return -1;
> case 0:
> /* Child */
> init_asl_core();
>
> /* stdout to be redirected down the writer end of pipe */
> - if (pipefds[1] != STDOUT_FILENO) {
> - if (dup2(pipefds[1], STDOUT_FILENO) < 0) {
> + if (stdout_fds[1] != STDOUT_FILENO)
> + if (dup2(stdout_fds[1], STDOUT_FILENO) < 0)
> + _exit(EXIT_FAILURE);
> + if (stderr_fds[1] != STDERR_FILENO)
> + if (dup2(stderr_fds[1], STDERR_FILENO) < 0)
> _exit(EXIT_FAILURE);
> - }
> - }
> /* Close reader end */
> - (void)close(pipefds[0]);
> + (void)close(stdout_fds[0]);
> + (void)close(stderr_fds[0]);
>
> /* Setup ACPICA compiler globals */
> Gbl_DisasmFlag = FALSE;
> @@ -128,32 +172,28 @@ int fwts_iasl_assemble_aml(const char *source, char **output)
> * We need to flush buffered I/O on IASL stdout
> * before we exit
> */
> - fflush(stdout);
> + (void)fflush(stdout);
> + (void)fflush(stderr);
>
> _exit(0);
> break;
> default:
> /* Parent */
> -
> /* Close writer end */
> - (void)close(pipefds[1]);
> + (void)close(stdout_fds[1]);
> + (void)close(stderr_fds[1]);
>
> - /* Gather output from IASL */
> - while ((n = read(pipefds[0], buffer, sizeof(buffer))) > 0) {
> - data = realloc(data, len + n + 1);
> - if (data == NULL) {
> - ret = -1;
> + while (!stdout_eof && !stderr_eof) {
> + if (fwts_iasl_read_output(stdout_fds[0], stdout_output, &stdout_len, &stdout_eof) < 0)
> + break;
> + if (fwts_iasl_read_output(stderr_fds[0], stderr_output, &stderr_len, &stderr_eof) < 0)
> break;
> - }
> - memcpy(data + len, buffer, n);
> - len += n;
> - data[len] = '\0';
> }
> (void)waitpid(pid, &status, WUNTRACED | WCONTINUED);
> - (void)close(pipefds[0]);
> + (void)close(stdout_fds[0]);
> + (void)close(stderr_fds[0]);
> break;
> }
>
> - *output = data;
> return ret;
> }
> diff --git a/src/acpica/source/compiler/fwts_iasl_interface.h b/src/acpica/source/compiler/fwts_iasl_interface.h
> index f88ccd8..71675d3 100644
> --- a/src/acpica/source/compiler/fwts_iasl_interface.h
> +++ b/src/acpica/source/compiler/fwts_iasl_interface.h
> @@ -21,6 +21,6 @@
> #define __FWTS_IASL_INTERFACE__
>
> int fwts_iasl_disassemble_aml(const char *aml, const char *outputfile);
> -int fwts_iasl_assemble_aml(const char *source, char **output);
> +int fwts_iasl_assemble_aml(const char *source, char **stdout_output, char **stderr_output);
>
> #endif
> diff --git a/src/lib/include/fwts_iasl.h b/src/lib/include/fwts_iasl.h
> index 58ffffa..78b2936 100644
> --- a/src/lib/include/fwts_iasl.h
> +++ b/src/lib/include/fwts_iasl.h
> @@ -24,7 +24,7 @@
>
> int fwts_iasl_disassemble_all_to_file(fwts_framework *fw);
>
> -int fwts_iasl_disassemble(fwts_framework *fw,
> +int fwts_iasl_disassemble(fwts_framework *fw,
> const char *table,
> const int which,
> fwts_list **ias_output);
> @@ -33,6 +33,7 @@ int fwts_iasl_reassemble(fwts_framework *fw,
> const uint8_t *data,
> const int len,
> fwts_list **iasl_disassembly,
> - fwts_list **iasl_errors);
> + fwts_list **iasl_stdout,
> + fwts_list **iasl_stderr);
>
> #endif
> diff --git a/src/lib/src/fwts_iasl.c b/src/lib/src/fwts_iasl.c
> index 5058c1e..27eeef0 100644
> --- a/src/lib/src/fwts_iasl.c
> +++ b/src/lib/src/fwts_iasl.c
> @@ -172,14 +172,17 @@ int fwts_iasl_reassemble(fwts_framework *fw,
> const uint8_t *data,
> const int len,
> fwts_list **iasl_disassembly,
> - fwts_list **iasl_errors)
> + fwts_list **iasl_stdout,
> + fwts_list **iasl_stderr)
> {
> char tmpfile[PATH_MAX];
> char amlfile[PATH_MAX];
> - char *output_text = NULL;
> + char *stdout_output = NULL, *stderr_output = NULL;
> int pid = getpid();
>
> - if ((iasl_disassembly == NULL) || (iasl_errors == NULL))
> + if ((iasl_disassembly == NULL) ||
> + (iasl_stdout == NULL) ||
> + (iasl_stderr == NULL))
> return FWTS_ERROR;
>
> fwts_acpcia_set_fwts_framework(fw);
> @@ -205,10 +208,10 @@ int fwts_iasl_reassemble(fwts_framework *fw,
>
> /* Now we have a disassembled source in tmpfile, so let's assemble it */
>
> - if (fwts_iasl_assemble_aml(tmpfile, &output_text) < 0) {
> + if (fwts_iasl_assemble_aml(tmpfile, &stdout_output, &stderr_output) < 0) {
> (void)unlink(amlfile);
> (void)unlink(tmpfile);
> - free(output_text);
> + free(stdout_output);
> return FWTS_ERROR;
> }
>
> @@ -220,8 +223,9 @@ int fwts_iasl_reassemble(fwts_framework *fw,
> snprintf(tmpfile, sizeof(tmpfile), "/tmp/fwts_iasl_%d.aml", pid);
> (void)unlink(tmpfile);
>
> - *iasl_errors = fwts_list_from_text(output_text);
> - free(output_text);
> + *iasl_stdout = fwts_list_from_text(stdout_output);
> + *iasl_stderr = fwts_list_from_text(stderr_output);
> + free(stdout_output);
>
> return FWTS_OK;
> }
>
Acked-by: Ivan Hu <ivan.hu at canonical.com>
More information about the fwts-devel
mailing list